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

feat: use predicted manifest update time to determine when to refetch #180

Merged
merged 4 commits into from
Nov 9, 2023

Conversation

kyle-cochran
Copy link
Contributor

Pipeline passing depends on the associated OSTk Data MR being merged:
open-space-collective/open-space-toolkit-data#6

Uses the "manifest" entry in the manifest to predict when the remote will have updated data and waits until then to poll for a new manifest file, rather than doing it every time.

Copy link
Contributor

@vishwa2710 vishwa2710 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM. Just 1 small comment on using C++20

@kyle-cochran kyle-cochran force-pushed the users/kyle/manifest-update-limit branch from d0a4204 to efee1e4 Compare November 6, 2023 20:36
Copy link
Contributor

@apaletta3 apaletta3 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Couple small comments! Great initiative and implementation.

src/OpenSpaceToolkit/Physics/Data/Manager.cpp Outdated Show resolved Hide resolved
src/OpenSpaceToolkit/Physics/Data/Manifest.cpp Outdated Show resolved Hide resolved
kyle-cochran and others added 4 commits November 9, 2023 19:43
Co-authored-by: Vishwa Shah <vishwa2710@gmail.com>
* refactor: remove unused in-memory caches from IERS data manager

* test: mock the fetch from manifest test

* test: fix IERS test load expecation

* build: add gmock linking to CMakeLists

* refactor: change get*At methods to get* and update bindings

* chore: remove test references to old methods

* fix: bug in BulletinA loader that loaded observation data as prediction

* test: fix expectations for test reset test functions since the getters auto-fetch

* test: fix expectations for test reset test functions since the getters auto-fetch

* style: formatting

* test: make file age test portable

* feat: add constness to mutex-protected functions

* refactor: rename IERS Manager private functions with underscores for clarity

* style: re-order IERS manager functions

* refactor: add suggestions from code review

* feat: Manifest Manager thread safety

* style: formatting

* test: update mock to match new function name
@kyle-cochran kyle-cochran force-pushed the users/kyle/manifest-update-limit branch from d7ee04e to f6bebc1 Compare November 9, 2023 19:44
@kyle-cochran kyle-cochran enabled auto-merge (squash) November 9, 2023 19:51
Copy link

codecov bot commented Nov 9, 2023

Codecov Report

Merging #180 (f6bebc1) into main (0b4e207) will increase coverage by 0.18%.
The diff coverage is 86.88%.

@@            Coverage Diff             @@
##             main     #180      +/-   ##
==========================================
+ Coverage   80.00%   80.18%   +0.18%     
==========================================
  Files          93       94       +1     
  Lines        7460     7458       -2     
==========================================
+ Hits         5968     5980      +12     
+ Misses       1492     1478      -14     
Files Coverage Δ
src/OpenSpaceToolkit/Physics/Data/Manifest.cpp 98.03% <100.00%> (+0.31%) ⬆️
...sics/Coordinate/Frame/Providers/IERS/BulletinA.cpp 89.47% <83.33%> (-0.16%) ⬇️
...cs/Coordinate/Frame/Providers/IERS/Finals2000A.cpp 60.43% <85.71%> (-0.36%) ⬇️
src/OpenSpaceToolkit/Physics/Data/Utilities.cpp 88.88% <88.88%> (ø)
src/OpenSpaceToolkit/Physics/Data/Manager.cpp 77.24% <82.60%> (-2.23%) ⬇️
...hysics/Coordinate/Frame/Providers/IERS/Manager.cpp 80.46% <87.75%> (+4.55%) ⬆️

@kyle-cochran kyle-cochran merged commit 9eaf267 into main Nov 9, 2023
11 checks passed
@kyle-cochran kyle-cochran deleted the users/kyle/manifest-update-limit branch November 9, 2023 19:59
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.

3 participants