-
Notifications
You must be signed in to change notification settings - Fork 80
Fix timeout preventing storage of TLEs #213
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
Conversation
mraspaud
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM, just one question inline
| source, len(failures), ", ".join(failures)) | ||
| logging.info("Downloaded %d TLEs from %s", | ||
| len(tles[source]), source) | ||
| logging.info("Downloaded %d TLEs from %s", len(tles[source]), source) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should we have a check that raises an error if none of the sources worked?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
No, I don't think so. If there's no data the database is not updated, whether it is due to network errors or the database already having the newest data stored.
|
I'll have a look at the CI failure, which I didn't get locally... |
|
Well, I did have the failures also locally, just didn't run the tests after adding |
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #213 +/- ##
==========================================
+ Coverage 90.35% 90.45% +0.10%
==========================================
Files 19 19
Lines 3027 3039 +12
==========================================
+ Hits 2735 2749 +14
+ Misses 292 290 -2
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
If there was a timeout downloading TLEs from for example Celestrak, the http timeout raised an exception and the
fetch_tles.pyscript terminated without saving any TLEs even if they were available from other sourves. This PR changes the exception to a logged error.