Skip to content

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.

Sign up for GitHub

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

Merged
merged 1 commit into from
Sep 17, 2025
Merged

Change the location of the FIRST cutout server #3413

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion astroquery/image_cutouts/first/__init__.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@ class Conf(_config.ConfigNamespace):
Configuration parameters for `astroquery.image_cutouts.first`.
"""
server = _config.ConfigItem(
['https://third.ucllnl.org/cgi-bin/firstcutout'],
['https://sundog.stsci.edu/cgi-bin/firstcutout'],
Copy link
Member

@bsipocz bsipocz Sep 17, 2025

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.

Copy link
Contributor Author

@rlwastro rlwastro Sep 17, 2025

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.

Copy link
Member

@bsipocz bsipocz Sep 17, 2025

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)

'Name of the FIRST server.')
timeout = _config.ConfigItem(
60,
Expand Down
Loading