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

Implement a cgi alive test when determining what VSO endpoint to use #6362

Merged
merged 8 commits into from Apr 28, 2023

Conversation

Cadair
Copy link
Member

@Cadair Cadair commented Aug 15, 2022

fixes #6361

I have no idea how to test this, as we are testing that a url in a remote file is accessible. I guess the only way would be to trick the function into downloading a pre-faked WSDL file.

sunpy/net/vso/vso.py Outdated Show resolved Hide resolved
@alasdairwilson
Copy link
Member

Thanks for this, very edge case but frustrating to encounter. 👍

@Cadair Cadair added BugFix net Affects the net submodule labels Aug 16, 2022
@Cadair Cadair marked this pull request as ready for review August 16, 2022 08:41
@Cadair Cadair requested a review from a team as a code owner August 16, 2022 08:41
sunpy/net/vso/vso.py Show resolved Hide resolved
sunpy/net/vso/vso.py Show resolved Hide resolved
sunpy/net/vso/vso.py Show resolved Hide resolved
@dstansby dstansby marked this pull request as draft September 12, 2022 13:15
@nabobalis nabobalis added this to the 4.1.0 milestone Sep 28, 2022
@nabobalis nabobalis removed this from the 4.1.0 milestone Oct 17, 2022
@dstansby
Copy link
Member

Nudge - I'm happy to try and finish this up @Cadair if you don't have time?

@nabobalis nabobalis modified the milestones: 4.1.4, 5.0.0 Jan 31, 2023
@nabobalis nabobalis removed this from the 5.0.0 milestone Feb 8, 2023
@nabobalis nabobalis added Minor Change PR only needs one approval to merge and removed BugFix labels Mar 6, 2023
@nabobalis
Copy link
Contributor

I wasn't sure what was left for this but I rebased and added some mock tests for the new functions.

I think a better mocked test would be to make the first urlopen fail then see if it connects to a second URL later on as a real remote test.

@Cadair Cadair marked this pull request as ready for review April 26, 2023 14:27
@Cadair Cadair requested a review from dstansby April 26, 2023 14:27
@Cadair
Copy link
Member Author

Cadair commented Apr 26, 2023

The online test fails are unrelated

@nabobalis
Copy link
Contributor

LGTM, shall we merge?

@nabobalis nabobalis added backport 5.0 on-merge: backport to 5.0 and removed backport 5.0 on-merge: backport to 5.0 labels Apr 28, 2023
@nabobalis nabobalis removed the request for review from dstansby April 28, 2023 21:51
@nabobalis nabobalis dismissed dstansby’s stale review April 28, 2023 21:51

Should all be fixed.

@nabobalis nabobalis merged commit b303fd9 into sunpy:main Apr 28, 2023
24 of 26 checks passed
@Cadair Cadair deleted the zeep_service_url_test branch April 30, 2023 13:19
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Minor Change PR only needs one approval to merge net Affects the net submodule
Projects
None yet
4 participants