-
Notifications
You must be signed in to change notification settings - Fork 243
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
concurrent projects polling; syntax fixes #88
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 a lot for this thorough revision of the logic @inge4pres! I have learnt a lot of things 🙏🙇♂️
I made a few comments on which we can discuss further but on overall I am happy to see it merged 👍
…l_seconds` and corresponding attribute in Config field
…ntax; write changelog
Hey there @mvisonneau I see this PR is growing a lot! all the discussions should be addressed, please have a look 😄 I updated the changelog with the content of the changes |
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 a lot @inge4pres, this looks really good! I just left a small comment but otherwise LGTM 👍
0e3acd4
to
3569450
Compare
…jects polling, adding tests
Hello,
here are addressed some of the design improvements highlighted in #86.
The main area is around pollProjects architecture that is now fully concurrent compared to the previous long-polling version.
There are also some minor cosmetic stuff like avoiding init() for metrics registry and some syntax simplifications.