Issue #96: Add port support #97
Conversation
|
/cc @RussTheAerialist @geordielad |
| self._port = value | ||
| # If port is None we don't actually want to update the XML | ||
| if value is not None: | ||
| self._connectionXML.set('port', value) |
There was a problem hiding this comment.
How do you remove the port if it exists and you want to remove it?
There was a problem hiding this comment.
Hmm. I can invert the logic so that it's:
@port.setter
def port(self, value):
self._port = value
# If port is None we remove the element and don't write it to XML
if value is None:
try:
self._connectionXML.attrib.pop('port')
except KeyError:
pass
else:
self._connectionXML.set('port', value)
EDIT: This is an implementation that passes tests
There was a problem hiding this comment.
That would be good, though I prefer this semantic for deleting rather than poping
del self._connectionXML.attrib['port']There was a problem hiding this comment.
Done -- one SO post said del doesn't work, but others said it's fine and it works for me too
| @@ -30,18 +30,20 @@ def __init__(self, connxml): | |||
| self._username = connxml.get('username') | |||
| self._authentication = connxml.get('authentication') | |||
| self._class = connxml.get('class') | |||
| self.port = connxml.get('port', None) | |||
There was a problem hiding this comment.
should this be self._port?
There was a problem hiding this comment.
I think originally I didn't so that validation logic would get triggered, but with my new "Don't write it" approach I think I can do just _port now.
Addresses #96
Adding support for reading the
portattribute from connections that have one.We keep
Nonein the model for cases where there is no port, but that won't get written to the XML.Setting it to any other value will be persisted in the XML
This is a minimal change to resolve the issue -- I realize that we could do some cleanup on class construction like in the client-python-lib, but I don't think it's necessary in this PR