Skip to content
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

Issue984 flow run refactor (branches from the other polling PR) #990

Closed
wants to merge 18 commits into from

Conversation

petersilva
Copy link
Contributor

poll can now run download like all the other components... kind of a happy side effect of just refactoring the code to make it easier to understand.

builds on #985

The run() routine is 288 lines ... it is too long to think about. Refactored code chunks into separate routines
just to shorten the code in run() The code didn't change much at all, it is just re-arranged.
Moved:

  • took all the message reformatting, callback invocation and worklist acknowledgements around the do() routing and created
    a work() function as a wrapper for it.
  • similarly moved the callback invocation and ack and worklist changes and from run() to the end of the filter() function.
  • did similar thing for the post() routine moving stuff from run() into post.
  • made a _run_vip_update routine outside of run() but called from it.
    • now could move on_start callback invocation so that run_vip_update() is called from a single place.

run() is now 159 lines. Changes in behaviour:

  • when poll has the vip it calls do(), so similar to all other components it can do a download in a single component if desired.
  • now novipfile management done in one place, also calls to housekeeping processing reduced from 2 to 1.

This PR includes the PR it is based on since that one isn't merged yet... will see how it goes if the other one gets merged first.

@petersilva
Copy link
Contributor Author

Probably best to merge the other one, then start this over... because I want a single squash merge for the other one, but in this one, every change is ok (none break the tests.) so it's ok to keep them separate.

@petersilva
Copy link
Contributor Author

will close for now.

@petersilva petersilva closed this Mar 21, 2024
@petersilva petersilva deleted the issue984_flow_run_refactor branch May 7, 2024 17:53
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant