FIX: switching to https for sdss to avoid query issues

[bsipocz]

To fix #2644

Thanks @weaverba137 for the suggestion, this does the trick.

[bsipocz]

Before merging, we need to make sure to pay a bit closer attention on the test coverage for older DRs, and whether everything keeps working for those.

[codecov]

Codecov Report

Merging #2654 (315c83f) into main (8b24c19) will not change coverage.
The diff coverage is n/a.

@@           Coverage Diff           @@
##             main    #2654   +/-   ##
=======================================
  Coverage   68.94%   68.94%           
=======================================
  Files         304      304           
  Lines       22621    22621           
=======================================
  Hits        15596    15596           
  Misses       7025     7025           

Impacted Files	Coverage Δ
astroquery/sdss/__init__.py	100.00% <ø> (ø)

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

[weaverba137]

The changes look good, but here's some follow up from the SDSS data team that might be worth thinking about.

The load balancers are setup with a standard 301 redirect to https when clients connect using http. However, most http clients see 301 redirects and ( surprisingly ) drop the body, headers, and arguments of the original request ( and sometimes change their HTTP method ). This is why when you queried over http, you were getting a non-existent key stack trace: your original arguments were not being forwarded to a server after a 301 redirect.

Clearly if the query still works, then the payload is not being dropped, but it might even be worth testing both http and https separately to be absolutely sure. Maybe for only a few releases, so it doesn't add a huge number of tests.

[bsipocz]

This is eyeopening, thanks for sharing their response.

Clearly if the query still works, then the payload is not being dropped, but it might even be worth testing both http and https separately to be absolutely sure. Maybe for only a few releases, so it doesn't add a huge number of tests.

if we change the default url for https, then I think we'll be good without testing http behaviour. If a user changes the default, they are on their own to make sure that request actually works and makes sense 🤷‍♀️

[pllim]

Makes sense. Thanks!

[pllim]

Thanks! Any plans to push a pre-release out to PyPI?

[bsipocz]

Sure, I'll merge a few more things and push one at the end of the day.

[pllim]

I can confirm that the error went away downstream. Thanks again!