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

ENH: Cloud access #369

Closed
wants to merge 43 commits into from
Closed

ENH: Cloud access #369

wants to merge 43 commits into from

Conversation

bsipocz
Copy link
Member

@bsipocz bsipocz commented Oct 5, 2022

This is to bring in the fornax projects cloud_access prototype. At the time of opening the PR only does the migration of the code along with it's history, I'll still have to:

  • add tests
  • add documentation
  • work the code into pyvo, using the prototype decorators, etc.
  • work it into downstream, or at least use it in demo/prototype notebooks (e.g. inside fornax or astroquery) to showcase it works as expected.
  • move _download_file from astroquery.query instead of using astropy.utils.data

zoghbi-a and others added 30 commits March 4, 2022 08:38
Added a use-cases notebook and a top level function to manage calls to different classes.
returning the handler to be able to download the data itself
@bsipocz bsipocz added this to the v1.5 milestone Oct 5, 2022
@codecov
Copy link

codecov bot commented Oct 5, 2022

Codecov Report

Merging #369 (6343d9e) into main (80a807e) will decrease coverage by 2.50%.
The diff coverage is 13.90%.

@@            Coverage Diff             @@
##             main     #369      +/-   ##
==========================================
- Coverage   78.56%   76.06%   -2.51%     
==========================================
  Files          47       48       +1     
  Lines        5562     5787     +225     
==========================================
+ Hits         4370     4402      +32     
- Misses       1192     1385     +193     
Impacted Files Coverage Δ
pyvo/utils/cloud_access.py 13.90% <13.90%> (ø)
pyvo/dal/tap.py 72.91% <0.00%> (-0.26%) ⬇️
pyvo/registry/regtap.py 78.94% <0.00%> (ø)
pyvo/utils/vocabularies.py 66.66% <0.00%> (+2.38%) ⬆️

📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more

@msdemlei
Copy link
Contributor

msdemlei commented Oct 5, 2022

Hm... I'll not quarrel a lot, but I'm not totally overjoyed to see a significant amount of code against a proprietary service (AWS) in pyVO, and for all I can see there's no discernable initiative to actually create a standard for whatever is being done here.

So... I get it right that you'd like to have it in pyVO (rather than, say, astroquery) because it is used in connection with SIAP, and there are SIAP services out there that don't work without this code? If so, could the people pushing that plan perhaps at least post something on the DAL list explaining what they're trying to do?

@bsipocz
Copy link
Member Author

bsipocz commented Oct 5, 2022

@msdemlei - Yes, we'll do exactly that, running this prototype through the right channels. Let me do the code and docs development first before we jump in and start discussing standards, etc.

And it has been discussed a bit already, the code belongs to pyvo and not to astroquery, it supports a wide use case base, and it is certainly backend.

@trjaffe
Copy link
Contributor

trjaffe commented Oct 5, 2022

Hm... I'll not quarrel a lot, but I'm not totally overjoyed to see a significant amount of code against a proprietary service (AWS) in pyVO, and for all I can see there's no discernable initiative to actually create a standard for whatever is being done here.

So... I get it right that you'd like to have it in pyVO (rather than, say, astroquery) because it is used in connection with SIAP, and there are SIAP services out there that don't work without this code? If so, could the people pushing that plan perhaps at least post something on the DAL list explaining what they're trying to do?

Indeed, NAVO plans to bring this up at the Interop and describe what our needs and current ideas are and to ask for feedback. Many of us prefer to play with something while we think about it, and there is apparently precedent for experimental code beyond the standard being put into pyvo.

As for your other question, no, there are no SIAP services out there that do NOT work without this code. That's a lot of negatives; in other words, it is something we are trying with a few alt SIA services that return extra information, and this bit of code doesn't do anything without that information.

@bsipocz
Copy link
Member Author

bsipocz commented Dec 15, 2023

While this PR nicely kept the upstream commit history, the content is superseded by #489 and two other PRs in the plans, so I'm closing it for now.

@bsipocz bsipocz closed this Dec 15, 2023
@bsipocz bsipocz removed this from the v1.5 milestone Dec 15, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants