-
Notifications
You must be signed in to change notification settings - Fork 136
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
Refactor of Pollable for modularization effort #118
Conversation
Making PollAction a method of Pollable will allow us to avoid a circular import when we split out packages from the main containerbuddy package.
I've also moved some of the parsing functions into their own package so that I can share them with packages that implement Pollable but want to validate their inputs (which I suspect all our Pollable-implementing packages will want to do). |
fedd4d6
to
b4b3983
Compare
Packages that include Pollables are going to want to validation their own configuration, so by moving the parsing and IPs functions into a utils package we can do so without any circular references.
b4b3983
to
9b5ec8c
Compare
This is safe to merge and release as a point release, as we've added new public APIs to our package (not that anyone is using the |
@tgross I haven't been able to pull this branch and play with it yet, however extracting out the utils package seems like a good idea. Also the PollAction seems like an appropriate abstraction - so from that standpoint is looks good. I'm not sure how I'll be using it in the scheduled tasks, so I'd like to see if I can rebase on it and adapt what I have. |
Sounds good. Let's hold off merging this until we've both had a chance to try it out in the features we're working on. |
Alternately, if we decide we're happy with #123 that'll merge this as well. |
I'm going to close this PR because it's covered by #123 |
Making PollAction a method of Pollable will allow us to avoid a circular import when we split out packages from the main containerbuddy package.
@justenwalker this follows from the design I was describing in #83 (comment), which I think will make building the metrics and tasks packages easier and make our life easier when we split up the fat containerbuddy package into smaller bits. (Note that this only required changing one test, which relied on binding variables from the enclosing scope in
poll
, so I'm reasonably confident this is the minimal intervention we can make.)cc @misterbisson