EUCLID: include new parameters in the constructor to set the urls for the tap, datalink and cutout services

[cosmoJFH]

Dear astroquery team,

in order to get access to non-standard Euclid environments (develop and integration environments) throughout astroquery, we would like to include new parameters in the constructor of the class EuclidClass, that defines URLs for these particular environments.

cc @esdc-esac-esa-int
jira: EUCLIDSWRQ-191

[codecov]

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 69.38%. Comparing base (4c2caa2) to head (511d058).
Report is 7 commits behind head on main.

Additional details and impacted files

@@           Coverage Diff           @@
##             main    #3288   +/-   ##
=======================================
  Coverage   69.37%   69.38%           
=======================================
  Files         232      232           
  Lines       19689    19695    +6     
=======================================
+ Hits        13659    13665    +6     
  Misses       6030     6030           

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

[bsipocz]

I would bring this into an overall astroquery API discussion first. Using the config system works, but may not be ideal or overall convenient, so I would like to explore use cases and pro-con points before start patching up modules with workarounds.

[bsipocz]

This is already possible to use these optionals via the config system (I did the very same for the IRSA euclid testing prior to the data release).

Have you tried that and found not convenient or didn't know about it?
( I would like to think about the overall astroquery API first before starting patching up the modules with different approaches to override default config items)

[cosmoJFH]

In the init.py file we defined those environments that are exposed to the public (see dictionary ENVIRONMENTS). But we do not want to include similar information for more restricted environments (develop and integration environments), I mean, for those that should be only used by the develop team at ESAC.

[bsipocz]

What I mean is that you can have those locally in your astroquery.cfg file. It's not exposed to anyone. (But I found it a bit cumbersome as there can only be one config/url, so when I was testing on the IPAC side, I stuck with using the irsadev server for all my queries, and needed to remember to turn it back off to the defaults when I was testing non-euclid related things.

[bsipocz]

some docs is here: https://astroquery.readthedocs.io/en/latest/#default-configuration-file

[cosmoJFH]

Hi @bsipocz, thanks for your feedback. I didn't know the usage of the astroquery.cfg file. We have used it in a local implementation of the EuclidClass and it works fine. But you are right that any user that uses this mechanism needs to remember to turn back off to the default values.

An alternative is the usage of the handlers, as it was mentioned by @andamian

from astroquery.utils.tap import TapPlus
from astroquery.utils.tap.conn.tapconn import TapConn
from astroquery.esa.euclid import EuclidClass


tap_conn = TapConn(ishttps=True,
                              host='https://eas.esac.esa.int/',
                              server_context='tap-server',
                              tap_context="tap",
                              upload_context="Upload",
                              table_edit_context="TableTool",
                              data_context="data",
                              datalink_context="tap_plus_conn_handler")

datalink_handler = TapPlus(url='https://eas.esac.esa.int/', server_context="sas-dd", tap_context="tap-server", upload_context="Upload", table_edit_context="TableTool", data_context="data", datalink_context="datalink",  verbose=True,  client_id='ASTROQUERY', use_names_over_ids=True)

cutout_handler = TapPlus(url='https://eas.esac.esa.int/', server_context="sas-cutout", tap_context="tap-server", upload_context="Upload", table_edit_context="TableTool", data_context="cutout", datalink_context="datalink",  verbose=true,  client_id='ASTROQUERY', use_names_over_ids=true)


euclid = EuclidClass(tap_plus_conn_handler=tap_conn, datalink_handler=datalink_handler, cutout_handler=cutout_handler)

but we think that this could be too complex for a regular user.

[bsipocz]

Pinging a couple of API stakeholders. Overriding URLs that are defined in the __init__/config is possible in the central astroquery.cfg; but I admit it's not super convenient.
But I would like to approach it uniformly, and if coming up with a fix, fix it up for all the modules.

cc-ing API stakeholders who I think may have opinions/inputs on this: @keflavich @jespinosaar @ManonMarchand @andamian; but feel free to bring in more voices, too

[andamian]

I wouldn't complicate the API with something that it's only meant to be used in testing. It seems to me that the *_handlers are meant for this. Why not using them for this kind of internal testing?

[ManonMarchand]

On the current situation, we see that the mirrors of our different services are clearly under-used. But that could be a documentation issue? I just checked, and we don't mention in SIMBAD and VizieR's narrative doc that mirrors exist.

For local testing purposes, git patches are very nice to switch to internal test servers.

[bsipocz]

On the current situation, we see that the mirrors of our different services are clearly under-used. But that could be a documentation issue?

I think documentation is just parts of it, honestly if something works out of the box, I doubt that people would dive in and switch to use a different mirror on their own. That being said, mirrors should be mentioned in the docs, preferably with an example for their usage.

[hectorcanovas]

Dear all,

Thanks for this discussion. I am joining this thread as Support Archive Scientist to the (ESA) Gaia and Euclid archives.
Before stating my opinion about the proposal being discussed, I would like to make sure that I fully understand it:
Does the proposal above implies that every time that a user wants to access an internal environments (e.g., "pre-production" or "development"), the Default Configuration file must be updated accordingly?

If the answer to my question is "Yes", then I would strongly oppose to this proposal for the following reason. This implementation would unnecessarily complicate the daily work of many members of both consortia (Euclid and Gaia) who e.g. need to have access to these internal environments for different purposes (like validation of internal products, testing of new features and functionalities, etc. etc).

But please note that I am just sharing my opinion as Support Archive Scientist (and not as Astropy/Astroquery expert).

Regards,
Héctor Cánovas

[keflavich]

No, you don't have to update the config every time. With the config system, you need to do something like:

from astroquery.esa.euclid import conf, EuclidClass
conf.ENVIRONMENTS['<environment>']['url_server'] = <new_url>
euclid = EuclidClass(environment='<environment>')

which is particularly clunky, though if properly pre-set-up with URLs appropriately tied to environment, it looks convenient enough.

With this PR, we're adding more parameters to EuclidClass to specify other URLs. What I don't like is the mixture: we should not have some URLs specified via conf and others specified via keyword; that's confusing. We have a broader conversation about whether configuration-based or keyword-based is better for users, and there are arguments both ways, but looking at this case in particular, where each environment requires many URLs, I think the conf system is possibly better. It feels awkward to have to do, e.g.:

euclid = EuclidClass(euclid_tap_server=..., euclid_data_server=..., euclid_cutout_server=...,)

vs the current, reasonably elegant (if properly preconfigured),

euclid = EuclidClass(environment='dev')

[hectorcanovas]

Dear Adam,

Thanks a lot for the detailed explanation and quick reaction. I do agree that the current configuration via environment keyword is more elegant and simple for the user. We will get back to this somewhere next week (tomorrow and the day after are bank holidays in Spain).

Regards,
Héctor

[jespinosaar]

Hi all,

I agree that having too many parameters in a method may be a problem, so I would try to simplify this. If the only difference for the handlers is the URL, I would try to create a simple way to set it instead of adding more and more parameters. Not only for simplicity, but also for maintainability.

If the commands provided by @keflavich work, I would go for that option.

But, on the other hand, it is possible that the Euclid team does not want the URLs to be publicly available. Then I would provide a simple constructor pointing to the final URL and then offer the users the possibility of changing the URL, something like:

changeUrl(euclid_url)

So the constructor is executed again, using kwargs, and the handlers are reinitialized with the new URL. Of course, in that case, users should know the URLs they should point to. But this may be a simple approach as well.

@cosmoJFH , @hectorcanovas , maybe this is interesting for other missions as well, so we can discuss a common approach for different missions.

[keflavich]

You don't need to make the URLs available. You could edit your config locally, or just add a few lines of code in your tests setting up the conf within your script.

Is it necessary to specify every URL, or is it just the host server that's changing? The latter is more common. Then it's a much easier fix, e.g.:

euclid = Euclid(server_url='http://my.server.com')

and then have it set up the server suffixes (/tap, /data, etc) automatically if those are all the same. Is that perhaps the case?

[keflavich]

To put it another way, based on https://github.com/astropy/astroquery/pull/3288/files/511d058158f5112815092e3e0fcad2cbc0434b69#r2044111848, is it things like server_context that need to change, or whole URLs, or all of the above, when you switch to development servers?

[bsipocz]

I opened #3293 for tracking the doc issues of the config system.

[cosmoJFH]

Hi @keflavich, the solution you proposed is really fancy. It works like a charm:

>>> from astroquery.esa.euclid import conf, EuclidClass
>>> conf.ENVIRONMENTS['DEV']= {'url_server': 'https://easdev.esac.esa.int/', 'main_table': 'catalogue.mer_catalogue', 'main_table_ra_column': 'right_ascension', 'main_table_dec_column': 'declination'}
>>> euclid = EuclidClass(environment='DEV',verbose=True)
Created TAP+ (v20200428.1) - Connection:
	Host: easdev.esac.esa.int
	Use HTTPS: True
	Port: 443
	SSL Port: 443
Created TAP+ (v20200428.1) - Connection:
	Host: easdev.esac.esa.int
	Use HTTPS: True
	Port: 443
	SSL Port: 443
Created TAP+ (v20200428.1) - Connection:
	Host: easdev.esac.esa.int
	Use HTTPS: True
	Port: 443
	SSL Port: 443

[bsipocz]

OK, then I suggest to close this PR without merge, and add some to the docs. The above example can live in the Euclid docs (so consortium scientists don't need to dig too deep) as well as can be part of the more generic, config related docs, too.