Use dictionary config interfaces for publisher and subscriber creation #127
Conversation
Codecov Report
@@ Coverage Diff @@
## main #127 +/- ##
==========================================
+ Coverage 76.57% 77.84% +1.27%
==========================================
Files 18 18
Lines 3893 3990 +97
==========================================
+ Hits 2981 3106 +125
+ Misses 912 884 -28
Flags with carried forward coverage won't be shown. Click here to find out more.
Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here. |
trollmoves/server.py
Outdated
| @@ -428,7 +428,22 @@ def __init__(self, attrs, publisher): | |||
|
|
|||
| def run(self): | |||
| """Start listening to messages.""" | |||
| with Subscribe('', topics=self.attrs['listen'], addr_listener=True) as sub: | |||
| nameserver = self.attrs.get('nameserver') | |||
| if nameserver in ('false', 'False'): | |||
There was a problem hiding this comment.
I'll move FALSY and TRUTHY to __init__ and use them from there where needed.
trollmoves/server.py
Outdated
| with Subscribe( | ||
| services=self.attrs.get('services', ''), | ||
| topics=self.attrs.get('topics', self.attrs['listen']), | ||
| addr_listener=bool(self.attrs.get('addr_listener', True)), | ||
| addresses=addresses, | ||
| timeout=int(self.attrs.get('timeout')), | ||
| translate=bool(self.attrs.get('translate', False)), | ||
| nameserver=nameserver, | ||
| ) as sub: |
There was a problem hiding this comment.
Is there a test to check at least that the old behaviour is unchanged given the same config?
There was a problem hiding this comment.
Asking because codecov says that parts of this function are untested...
There was a problem hiding this comment.
Seems that the posttroll notifier (trollmoves.server.Listener class) has no tests. I'll see if I can figure out enough of it to create a test to see the default usage hasn't changed.
There was a problem hiding this comment.
I added the bare minimum test for this.
trollmoves/client.py
Outdated
| if nameservers in FALSY: | ||
| nameservers = False |
There was a problem hiding this comment.
Codecov says this is not tested?
|
I refactored the code and config reading a bit so that adding YAML config support would be a bit easier. |
This PR makes it possible to:
nameserverconnections by definingnameservers = False(Client) ornameserver = Falseandaddresses = srvr1:12345 srvr2:12345(Server, when using posttroll notifier) in the configuration fileNSSubscriberparameters by defining them in the Server configuration file when using posttroll notifierThe default behavior isn't affected.