Refactor dataservice #548
Merged
Refactor dataservice #548
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
aaraney
force-pushed
the
refactor-dataservice
branch
2 times, most recently
from
0c08d8f
to
d184db0
Compare
aaraney
force-pushed
the
refactor-dataservice
branch
from
c9fe53b
to
fa02c7b
Compare
aaraney
force-pushed
the
refactor-dataservice
branch
from
fa02c7b
to
2199895
Compare
christophertubbs
previously requested changes
Mar 15, 2024
Extract long running ServiceManager tasks into their own objects: - RequiredDataChecksManager - TempDatasetsManager - DataProvisionManager `Count` synchronization type added to facilitate cross-task signaling.
- move main function to of file for readability
- setup and inject subsystem dependencies
- reintroduce init_{{subsystem}}_helper methods once on ServiceManager
to private functions
- explicitly fail when service does not use object store. this was
previously a bug.
aaraney
force-pushed
the
refactor-dataservice
branch
from
76c8401
to
b9bc835
Compare
|
@robertbartel, just rebased. |
robertbartel
requested changes
Mar 27, 2024
| while True: | ||
| # Ensure that mark and sweep doesn't proceed while something is potentially to linking datasets | ||
| while self._tracker.value > 0: | ||
| await asyncio.sleep(10) |
There was a problem hiding this comment.
Should we encapsulate this in the ActiveOperationTracker class also?
|
Thanks for the reviews! Ill fix this up and have it ready for re-review in the morning. |
improve clarity of the utility of `ActiveOperationTracker` in doc strings and parameter names / instance variables.
aaraney
force-pushed
the
refactor-dataservice
branch
from
7f856a8
to
d5d87d3
Compare
|
Just pushed up resolutions for the requested improvements. This is ready for re-review now, @robertbartel! |
robertbartel
approved these changes
Mar 28, 2024
robertbartel
dismissed
christophertubbs’s stale review
Chris's requested changes have been made, as have mine, and I've separately approved.
|
Thanks, @robertbartel! |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
dmod.dataservice--0.8.2Changes
dataservicetasks into separate classes.