Changed Hub.registration_timeout to be a config input. #5303
Conversation
| @@ -410,7 +417,8 @@ def __init__(self, **kwargs): | |||
| """ | |||
|
|
|||
| super(Hub, self).__init__(**kwargs) | |||
| self.registration_timeout = max(10000, 5*self.heartmonitor.period) | |||
| if self.registration_timeout <= 0: | |||
There was a problem hiding this comment.
This type of validation should be done in a validation method _registration_timeout_changed method that traitlets will call automatically each time the value is set.
|
I put the validation there because the registration_timeout value needs to change when you set it OR when you set the hearmonitor period. If it's automatic, then what you get is order dependent. Of course I might not understand exactly how the traits work either... |
|
Then I think you should add validation logic to the traits handler for both of these attributes. The problem is that if you ever change the value after the class is created, the validation won't be performed. |
|
The traits duplication / validation is more complicated than it should be because I created the Factory stuff before we had config, and it mostly serves the same purpose (i.e. HubFactory does all validation and initialization for Hub). The trait does not need to be validated twice. Hopefully I will find the time to remove all of the Factory classes for 3.0. I made a PR against your branch (TD22057/ipython#1) to add the appropriate validation to registration_timeout. |
|
@TD22057 I made a PR against your branch. Do you want to merge that? If you aren't comfortable with that, I can finish this up in another PR. |
|
@minrk Sorry - I got yanked off this at work to finish something else and haven't had time to come back it. Given that I'd need to learn how to do the merge, it would be fantastic if you could take care of it. Or I can get back to this sometime next week if you want to wait - either way is fine with me. |
|
No problem, I'll finish it up. Thanks! |
|
will continue this PR in #5349. |
I did a quick test on my local machine with this. I couldn't figure out how to commit back to the github server so I did an online edit so someone should double check that everything is OK.