-
-
Notifications
You must be signed in to change notification settings - Fork 427
Change the location of the FIRST cutout server #3413
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
The old third server is no longer supported and has practically stopped working. This configuration change gives the new URL for the server. Note this change does not happen often -- the URL has been the same since 1997 before this change!
bsipocz
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.
Perfect timing as I'm tagging a release this week!
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #3413 +/- ##
=======================================
Coverage 70.51% 70.51%
=======================================
Files 232 232
Lines 19997 19997
=======================================
Hits 14100 14100
Misses 5897 5897 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
|
That's great news about the update this week! This will be broken until the updated config is available. (I tried to figure out how to tell users to modify the URL themselves, but that seems to require them to edit the configuration file. Run-time changes didn't work for me.) |
| """ | ||
| server = _config.ConfigItem( | ||
| ['https://third.ucllnl.org/cgi-bin/firstcutout'], | ||
| ['https://sundog.stsci.edu/cgi-bin/firstcutout'], |
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.
(I tried to figure out how to tell users to modify the URL themselves, but that seems to require them to edit the configuration file. Run-time changes didn't work for me.)
FWIW, for IRSA I routinely do run-time changes to swap between the dev/test and ops servers. The difference I see is that I don't have the value in a list. According to the astropy.config docs if the value is a list the behaviour is totally different 🤯
So, I would consider this as a bug for this module as we don't have mirrors here that we want to validate, but only one server with the option of maybe overriding the url.
I leave it up to you if you want to leave as is or change it to be not a list.
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.
I think it would be great if this could be changed by users! I'm nervous about making the change now though, since I think I inherited this config code from somewhere else and maybe there are other bits of code that rely on it.
I thought the list thing might be to blame -- I could see that the value type was listed as an "Option" rather than a string. Maybe I'll work on this after your release is done to improve the long term maintainability.
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.
OK, I did some more digging and this is a more widespread problem -- I opened #3414 to propose we clean up all the modules that doesn't explicitly work with mirrors.
(And I'm afraid you might have inherited this from the template module itself which shows how to deal with mirrors)
|
@rlwastro - not the cleanest, but this kind of works: |
The old third.ucllnl.org server is no longer supported and has practically stopped working. This configuration change gives the new URL for the server: sundog.stsci.edu.
Note this change does not happen often -- the URL has been the same since 1997 before this change!