-
Notifications
You must be signed in to change notification settings - Fork 2
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Add polling and test it #2
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks @Flix6x!
I have only minor change requests and a couple of potential new features:
- Using the same code to allow to retry the request in case of failiure (up to a maximum number of times.
- Limiting the polling time. It shouldn't occur, but if a Job becomes "Zombie" and it's not finishing, we just disregard the request. For instance, the polling time limit could be set to a few hours.
Signed-off-by: F.N. Claessen <[email protected]>
Signed-off-by: F.N. Claessen <[email protected]>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Good stuff. I have some ideas about the retrying intervals and, as always, naming.
Signed-off-by: F.N. Claessen <[email protected]>
Signed-off-by: F.N. Claessen <[email protected]>
Signed-off-by: F.N. Claessen <[email protected]>
Signed-off-by: F.N. Claessen <[email protected]>
Signed-off-by: F.N. Claessen <[email protected]>
Signed-off-by: F.N. Claessen <[email protected]>
Signed-off-by: F.N. Claessen <[email protected]>
Signed-off-by: F.N. Claessen <[email protected]>
Signed-off-by: F.N. Claessen <[email protected]>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks @Flix6x!
I've only small suggestions.
Other than that, I think we could add some validators to the FlexmeasuresClient
class, maybe not for this PR:
- lenght of emai, password and host
- force scheme to be
http
orhttps
- check if
polling_timeout > request_timeout > 0
Somewhat unrelated to this PR, but now that is early and we don't have too much depending on this, I was thinking that we might consider refactoring the class name from FlexmeasuresClient
to FlexMeasuresClient
.
Closing this PR, as its contents have been incorporated into #7. |
I implemented basic polling. Follow-ups:
get_schedule
method. I kind of like that the polling logic is implemented on all requests to FlexMeasures whose responses indicate to retry the request later. -> see Use Retry-After HTTP header for scheduling in progress flexmeasures#645