dbekaert / RAiDER

Raytracing Atmospheric Delay Estimation for RADAR
Apache License 2.0
69 stars 38 forks source link

Weather model credential checking will overwrite user netrc #638

Open jhkennedy opened 2 months ago

jhkennedy commented 2 months ago

In the RAiDER.models.credentials module, the check_api function will blow away a user's entire .netrc file, potentially removing access to other hosts. For example:

[1] from RAiDER.models.credentials import check_api
[2] !cat ~/.netrc
machine dataspace.copernicus.eu
        login foo
        password bar
machine urs.earthdata.nasa.gov
        login fizz
        password buzz 

[3] check_api('GMAO', 'flim', 'flam', update_flag=False)
[4] !cat ~/.netrc
machine dataspace.copernicus.eu
        login foo
        password bar
machine urs.earthdata.nasa.gov
        login fizz
        password buzz 

[5] check_api('GMAO', 'flim', 'flam', update_flag=True)
[6] !cat ~/.netrc
machine urs.earthdata.nasa.gov
        login flim
        password flam

Instead, it should load the credential file and modify only the urs.earthdata.nasa.gov host.

garlic-os commented 2 weeks ago

I am looking and found that RAiDER.s1_orbits.ensure_orbit_credentials does a lot of the same thing that RAiDER.models.credentials.check_api is meant to. Can somebody with more experience on the project explain the differences between these two functions?

jhkennedy commented 5 days ago

@garlic-os Yes, they do pretty much the same thing. From what I remember, RAiDER.models.credentials.check_api is coupled to the idea that you're checking credentials for the weather model API, but for GUNW processing, we needed to check for EDL credentials irrespective of the weather model as we'll always need them for the orbits.

Credential checking should probably be abstracted entirely from the model used and then called appropriately as needed, and then a general section in the docs on credentials provided (e.g., #614).

We were under a time crunch with an active processing campaign. It was easier to add the ensure_orbit_credentials, which we already had written elsewhere, than to spoof the model for orbits, and I didn't really have time to do the refactor (too many different file formats for all the credentials). I thought I had opened an issue for the refactor as well, but I don't see it.