Refactor DatasetSplitManager to reload latest cache. #527
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
4 tasks
gabegma
force-pushed
the
ggm/refactor-dm
branch
2 times, most recently
from
de74b94
to
3c40dd2
Compare
gabegma
force-pushed
the
ggm/improve-stability-start-up
branch
from
825ef39
to
bfa4299
Compare
gabegma
force-pushed
the
ggm/improve-stability-start-up
branch
3 times, most recently
from
a02a5bb
to
28a0a2d
Compare
gabegma
force-pushed
the
ggm/refactor-dm
branch
2 times, most recently
from
4402ef6
to
5f81471
Compare
gabegma
force-pushed
the
ggm/improve-stability-start-up
branch
from
4bba833
to
c28c839
Compare
gabegma
force-pushed
the
ggm/refactor-dm
branch
2 times, most recently
from
750cca7
to
a93803f
Compare
gabegma
force-pushed
the
ggm/refactor-dm
branch
2 times, most recently
from
4d33219
to
69b7949
Compare
JosephMarinier
changed the base branch from
main
to
ggm/fix-startup-dependencies
gabegma
force-pushed
the
ggm/fix-startup-dependencies
branch
from
0a8ed66
to
a9e900f
Compare
gabegma
force-pushed
the
ggm/refactor-dm
branch
from
69b7949
to
99178a8
Compare
- Define `get_time_from_file_name()` instead of duplicating the `.split("_")[-1][:-6]` logic.
- Use `except StopIteration` on `cache_file = next(...)` instead of passing a default value to `cache_file = next(..., None)` and then checking for `if not cache_file:`.
gabegma
commented
Apr 18, 2023
Co-authored-by: Joseph Marinier <joseph.marinier@servicenow.com>
JosephMarinier
approved these changes
Apr 18, 2023
Thank you for the quick review today and all of your contributions!! |
gabegma
added a commit
that referenced
this pull request
Apr 19, 2023
* Add pid to logs to help debugging
* Modify DM so it always load the latest cache
* Clean up `load_latest_cache()`
- Define `get_time_from_file_name()` instead of duplicating the `.split("_")[-1][:-6]` logic.
- Use `except StopIteration` on `cache_file = next(...)` instead of passing a default value to `cache_file = next(..., None)` and then checking for `if not cache_file:`.
* Apply suggestions from code review
Co-authored-by: Joseph Marinier <joseph.marinier@servicenow.com>
---------
Co-authored-by: joseph.marinier <joseph.marinier@servicenow.com>
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.
Part of #521
Description:
mod.get_dataset_split()first (or other dm methods which call it), for the DM to check if the dataset saved in memory is up-to-date. If they call some other methods before, I foresee it could cause an issue, but perhaps not? I haven't seen any issues with the tests not when starting the app though.Checklist:
You should check all boxes before the PR is ready. If a box does not apply, check it to acknowledge it.
ran
pre-commit run --all-filesat the end.our users.
READMEfiles and our wiki for any big design decisions, if relevant.