Implemented ConnectionParser to handle Federated and Legacy connections #7
Conversation
…o get all connection elements (that matter) and return them as a list. This is 'functional' and doesn't break old style connections. I'm going to try something in a different PR that might be a better approach
…t. I'm not sure if I like this approach or not.
| if float(self._dsversion) < 10: | ||
| connections = self._extract_legacy_connection() | ||
| else: | ||
| connections = self._extract_federated_connections() |
There was a problem hiding this comment.
I'm beginning to wonder if we shouldn't have a different class for the NamedConnection that just mimics the old interface, but can handle things like the caption and other attributes that legacy connections don't have.
But that might be more than we need right now
There was a problem hiding this comment.
I spoke to Sundeep, and he confirmed I can ignore the named-connection elements for Server purposes.
There was a problem hiding this comment.
Fine for now. In the end, we should have separate classes for parsing 10.0+ vs prior connections. This current format, I think, will lead to potentially a good number of if else cases. I think there can be inheritance. Just a note for going forward. For now ... I wouldn't bother.
…for now. Also added the ConnectionParser tests and added it to the __init__ file to make it importable. There are probaly redundant tests now, but they're still less than a second so oh well for now. ConnectionParser is responsible for extracting the connection elements from the Datasource XML. I'm debating making the interface consistent so that ._connection always returns a list, even if it's just one element... That can be another checkin
| @@ -28,7 +47,8 @@ def __init__(self, dsxml, filename=None): | |||
| self._datasourceTree = ET.ElementTree(self._datasourceXML) | |||
| self._name = self._datasourceXML.get('name') or self._datasourceXML.get('formatted-name') # TDS files don't have a name attribute | |||
| self._version = self._datasourceXML.get('version') | |||
| self._connection = Connection(self._datasourceXML.find('connection')) | |||
| self._connection_parser = ConnectionParser(self._datasourceXML, version=self._version) | |||
| self._connection = self._connection_parser.get_connections() | |||
There was a problem hiding this comment.
If we are returning a list, connection needs to be plural (same for the accessor)
There was a problem hiding this comment.
That breaks the existing API, but that's probably ok because only the sample needs to be updated :)
@benlower and I discussed always returning a list for connections, even in the legacy case, what do you think? I have that change shelved already and can add it to the PR.
There was a problem hiding this comment.
The "existing" api doesn't exist yet. :) I think we are okay for just a bit longer and then will have to worry about versioining and such but (a) we are still in BETA and (b) we haven't even released the BETA. :)
And I am good with returning a list even for older datasources. Consistent API. I don't want users having to put if cases for version numbers.
Addresses #5
Subsumed by later comments