Ori.livneh has uploaded a new change for review.
https://gerrit.wikimedia.org/r/317823
Change subject: Drop support for the legacy configuration format
......................................................................
Drop support for the legacy configuration format
Per Mark, in Ib7e9691cec3b: "I think we should attempt to completely
kill the old syntax ASAP, and just use JSON from now on. There are few
if any external users to worry about, so why introduce more tech debt?
Let's keep it clean and simple and consistent now we still can."
Change-Id: I304669e8dcbbf2cd3fa6e87746e16298cf873a46
---
M TODO
M pybal/config.py
M pybal/test/test_config.py
3 files changed, 4 insertions(+), 60 deletions(-)
git pull ssh://gerrit.wikimedia.org:29418/operations/debs/pybal
refs/changes/23/317823/1
diff --git a/TODO b/TODO
index 6acfaa1..e0f81b4 100644
--- a/TODO
+++ b/TODO
@@ -12,6 +12,5 @@
from the realservers, e.g. for computing the weight of a realserver
* Reimplement IPVSManager using Netlink once the IPVS Netlink interface
has entered the kernel
-* Revisit the whole config parsing wrt consistency and security
* Syntax errors in server lists cause unhandled Deferreds, and appear
- to stop any further config rereads - this needs to be fixed.
\ No newline at end of file
+ to stop any further config rereads - this needs to be fixed.
diff --git a/pybal/config.py b/pybal/config.py
index 3c5b167..a00f81a 100644
--- a/pybal/config.py
+++ b/pybal/config.py
@@ -87,35 +87,9 @@
if not self.reloadTask.running:
self.startObserving()
- def parseLegacyConfig(self, rawConfig):
- """Parse a legacy (eval) configuration file."""
- config = {}
- for line in rawConfig.split('\n'):
- try:
- line = line.strip()
- if not line or line.startswith('#'):
- continue
- server = ast.literal_eval(line)
- host = server.pop('host')
- config[host] = {'enabled': server['enabled'],
- 'weight': server['weight']}
- except (KeyError, SyntaxError, TypeError, ValueError) as ex:
- # We catch exceptions here (rather than simply allow them to
- # bubble up to FileConfigurationObserver.logError) because we
- # want to try and parse as much of the file as we can.
- log.err(ex, 'Bad configuration line: %s' % line)
- continue
- return config
-
- def parseJsonConfig(self, rawConfig):
+ def parseConfig(self, rawConfig):
"""Parse a JSON pool configuration file."""
return json.loads(rawConfig)
-
- def parseConfig(self, rawConfig):
- if self.configUrl.endswith('.json'):
- return self.parseJsonConfig(rawConfig)
- else:
- return self.parseLegacyConfig(rawConfig)
def reloadConfig(self):
"""If the configuration file has changed, re-read it. If the parsed
diff --git a/pybal/test/test_config.py b/pybal/test/test_config.py
index d3930f8..bef147a 100644
--- a/pybal/test/test_config.py
+++ b/pybal/test/test_config.py
@@ -75,16 +75,6 @@
def testParseConfig(self):
"""Test `FileConfigurationObserver.parseConfig`"""
- self.observer.parseLegacyConfig =
mock.MagicMock(return_value='legacy_config')
- self.observer.parseJsonConfig =
mock.MagicMock(return_value='json_config')
- self.assertEquals(self.observer.parseConfig("I am legacy"),
'legacy_config')
- self.observer.parseLegacyConfig.assert_called_with("I am legacy")
- self.observer.configUrl += '.json'
- self.assertEquals(self.observer.parseConfig("I am json"),
'json_config')
- self.observer.parseJsonConfig.assert_called_with("I am json")
-
- def testParseJsonConfig(self):
- """Test `FileConfigurationObserver.parseJsonConfig`"""
json_config = """
{
"mw1200": { "enabled": true, "weight": 10 },
@@ -95,30 +85,11 @@
'mw1200': {'enabled': True, 'weight': 10},
'mw1201': {'enabled': False, 'weight': 1},
}
- self.assertEquals(self.observer.parseJsonConfig(json_config),
+ self.assertEquals(self.observer.parseConfig(json_config),
expected_config)
invalid_config = "{[]"
- self.assertRaises(Exception, self.observer.parseJsonConfig,
+ self.assertRaises(Exception, self.observer.parseConfig,
invalid_config)
-
- def testParseLegacyConfig(self):
- """Test `FileConfigurationObserver.parseLegacyConfig`"""
- legacy_config = '\n'.join((
- "{'host': 'mw1200', 'weight': 10, 'enabled': True }",
- "{'host': 'mw1201', 'weight': 1, 'enabled': False }",
- ))
- expected_config = {
- 'mw1200': {'enabled': True, 'weight': 10},
- 'mw1201': {'enabled': False, 'weight': 1},
- }
- self.assertEquals(self.observer.parseLegacyConfig(legacy_config),
- expected_config)
-
- invalid_config= "{'host': 'something'}\n"
-
- # Needed for nose to pass... it doesn't really get raised
- self.assertEquals(self.observer.parseLegacyConfig(invalid_config), {})
- self.flushLoggedErrors(KeyError)
class HttpConfigurationObserverTestCase(PyBalTestCase):
--
To view, visit https://gerrit.wikimedia.org/r/317823
To unsubscribe, visit https://gerrit.wikimedia.org/r/settings
Gerrit-MessageType: newchange
Gerrit-Change-Id: I304669e8dcbbf2cd3fa6e87746e16298cf873a46
Gerrit-PatchSet: 1
Gerrit-Project: operations/debs/pybal
Gerrit-Branch: master
Gerrit-Owner: Ori.livneh <o...@wikimedia.org>
_______________________________________________
MediaWiki-commits mailing list
MediaWiki-commits@lists.wikimedia.org
https://lists.wikimedia.org/mailman/listinfo/mediawiki-commits
