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

Refactor health manager #172

Open
wants to merge 8 commits into
base: stable/yoga-m3
Choose a base branch
from
Open

Conversation

BenjaminLudwigSAP
Copy link
Collaborator

@BenjaminLudwigSAP BenjaminLudwigSAP commented Aug 4, 2022

This PR is a follow-up to #171

  • Rename variables
  • add/fix docstrings/comments
  • reorder code, simplify conditional branching

TODO:

  • Simplify everything. Make it less generic, now that it's all in F5PD
  • Fix amphora cleanup (lots of orphaned amphora entries still lying around).

@BenjaminLudwigSAP BenjaminLudwigSAP force-pushed the healthmanager branch 4 times, most recently from 4569345 to dcb7060 Compare August 10, 2022 11:47
@BenjaminLudwigSAP BenjaminLudwigSAP force-pushed the healthmanager branch 2 times, most recently from 516966e to bacf7fd Compare August 12, 2022 08:16
NoResultFound has to be catched seperately, because it inherits
InvalidRequestError (which makes absolutely no sense, but whatever)
- Log once status manager is done initializing
- Rename variables to snake case
- Fix typos in docstrings
Also:
- Rename variables
- PEP8
- Remove wrong comment. 99496ec did
  state that it fixed a race condition but I think it was really just
  an error with non-existent and/or URL encoded member stats. Not
  quite sure though.
It's directly connected to amphora_messages, so move it right after
the declaration.
Add note in update_db.py for clarification and as placeholder for
rework of status manager mechanics.
@notandy
Copy link
Collaborator

notandy commented Aug 16, 2022

You assigned this PR for review, but it has open todos, should we wait for review till you implemented them?

@BenjaminLudwigSAP
Copy link
Collaborator Author

Sorry, I originally included a fix in this PR, but have since pushed that fix to stable/yoga-m3, so this is a mere refactoring PR now. No review required yet, but of course always welcome anyway :)

@BenjaminLudwigSAP BenjaminLudwigSAP self-assigned this Apr 12, 2023
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.

2 participants