-
Notifications
You must be signed in to change notification settings - Fork 0
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
Adding bandwith measurement to downloadCutout #9
Conversation
mtauraso
commented
Aug 12, 2024
- Stats tracking of timing, data size, and bandwidth for cutout transfer
- Ability to specify an arbitrary list of filters to the downloader
- Configurable offset and extent for downloading from a large .fits catalog
- download() helper function codepath now supports kwargs passed from download() -> _download() -> _download_chunk() -> urllib.request.urlopen()
- Timout and chunk size for downloadCutout.py are configurable via this kwargs system.
- Placeholder config for download verb - Stats tracking of timing, data size, and bandwidth for cutout transfer - Ability to specify an arbitrary list of filters to the downloader - Configurable offset and extent for downloading from a large .fits catalog - download() helper function codepath now supports kwargs passed from download() -> _download() -> _download_chunk() -> urllib.request.urlopen() - Timout and chunk size for downloadCutout.py are configurable via this kwargs system.
# TODO: See if this sort is performed by downloadCutouts | ||
# It appears downloadCutouts is doing some sorting prior to download, but | ||
# unclear if it is the same sort | ||
locations.sort(variable_fields) |
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.
This sorting turned out to be unnecessary.
The Rect
class in downloadCutout.py
has a very particular sort order imposed on it, and the download codepath calls Rect.explode()
under the hood, which calls .sort()
on a list of Rect
s particular to each request. This results in the server receiving lists sorted by Tract, RA, DEC and some other fields.
@@ -118,11 +120,77 @@ def create_rects(locations: Table, offset: int = 0, default: dC.Rect = None) -> | |||
return rects | |||
|
|||
|
|||
stats = { |
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.
This whole stats subsystem is pretty hacky. I've tried to make it unobtrusive to the CLI, but if we end up using it for anything beyond spot checks, we likely want to go find ourselves a library to do this.
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #9 +/- ##
========================================
- Coverage 9.83% 6.89% -2.94%
========================================
Files 6 6
Lines 61 87 +26
========================================
Hits 6 6
- Misses 55 81 +26 ☔ View full report in Codecov by Sentry. |
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.
This seems plenty fine to me!