Skip to content

Update credentials management#70

Merged
bgruening merged 34 commits intoNordicESMhub:masterfrom
Marie59:credentials
Mar 3, 2026
Merged

Update credentials management#70
bgruening merged 34 commits intoNordicESMhub:masterfrom
Marie59:credentials

Conversation

@Marie59
Copy link
Contributor

@Marie59 Marie59 commented Feb 13, 2026

Hi all !

I'm trying to do the update of this tool. I don't know how to inject the secret into the configfile to then get it the .cdsapirc.
Any ideas ?

Ping @bgruening @yvanlebras @PaulineSGN

@yvanlebras
Copy link

Hi Marie ! THANK YOU! If I am not saying a mistake, looking at this example https://github.com/galaxyecology/tools-ecology/pull/214/changes#diff-135d08b2c88f38c565c73c2f2ee959d66a4520c676eaa880c6b0f8d9a336d7e1 , one can remove the use of .cdsapirc and maybe configfile. Am I wrong ?

@PaulineSGN
Copy link
Contributor

Hi !
Yes, you don't need configfile anymore, since the api key is directly injected in the environment variable CDS_API_KEY.

@Marie59
Copy link
Contributor Author

Marie59 commented Feb 13, 2026

Yes but then we also need to inject the URL one way or another no ?

@PaulineSGN
Copy link
Contributor

Yes but then we also need to inject the URL one way or another no ?

Yes sorry, I didn't see that. Does it work manage in the python script ?

@Marie59
Copy link
Contributor Author

Marie59 commented Feb 13, 2026

For now no i'm still testing ;) But if you have any ideas don't hesitate

@Marie59
Copy link
Contributor Author

Marie59 commented Feb 16, 2026

@annefou Apparently the python version is not up to date (if I understand correctly). Does this repo need a full update ? If yes could it break other tools ?

What do you think ?
@bgruening do you have an opinion on this too ?

Btw, the tool is working locally :)

@bgruening
Copy link
Collaborator

Please update the CI script in a different PR. Copy the one from IUC to here and adopt it to this repo, e.g. changing the galaxyproejct/iuc

@Marie59 Marie59 mentioned this pull request Feb 16, 2026
@bgruening
Copy link
Collaborator

Ok, now all the errors and warnings seems legit and should be worked on ideally.

@Marie59
Copy link
Contributor Author

Marie59 commented Feb 17, 2026

I don't get the error about the tests. Do you have any ideas ?

Co-authored-by: PaulineSGN <107857379+PaulineSGN@users.noreply.github.com>
@yvanlebras
Copy link

So the idea here is to use this ""error message"" to test the execution of the command line who provides an error message from cdsapi COpernicus Python API package, waiting for Planemo to allow to use the new manner to manage credentials in Galaxy (if I am not wrong) so Planemo is not finding the Galaxy env variables related to credentials and try to find the cdsapi file and return an error on it. Am I ok @PaulineSGN @Marie59 ? Is it ok for you @bgruening ?


if path.isfile(cdapi_file):
c = cdsapi.Client()
c.retrieve(
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

should we not add here a proper try/except and raise an error that we can assert in the XML tests?

@bgruening
Copy link
Collaborator

This will also improve the situation at some point: galaxyproject/galaxy#21643

@Marie59
Copy link
Contributor Author

Marie59 commented Feb 19, 2026

Okay I guess everything is fix ! Except fort this @bgruening https://github.com/NordicESMhub/galaxy-tools/actions/runs/22145319976/job/64020879425?pr=70#step:6:20
Even thought I added the file it does not seem happy

@Marie59 Marie59 closed this Feb 24, 2026
@Marie59 Marie59 reopened this Feb 24, 2026
@annefou
Copy link

annefou commented Feb 25, 2026

I am not sure how to help @Marie59 We can definitely update the repo. We could also have only one joint repo for all our tools, (similar to what we now have with. the element channel)

@Marie59
Copy link
Contributor Author

Marie59 commented Feb 26, 2026

I am not sure how to help @Marie59 We can definitely update the repo. We could also have only one joint repo for all our tools, (similar to what we now have with. the element channel)

Hi @annefou I started to update this repo already (It should be up to date now). Yes at least for earth and climate this would be great to have a joint repo. Just need to think on how to that nicely ;)

@Marie59
Copy link
Contributor Author

Marie59 commented Mar 3, 2026

I don't know how to fix the issue here. It's working locally for me. Any ideas ? @bgruening

@bgruening
Copy link
Collaborator

Expected text 'CDS retrieval failed, make sure you filled in your CDS API Key' in output ('')

You assert for this given error message, but there is no error message. Maybe you are printing it to stdout? Or its not triggered?

@Marie59
Copy link
Contributor Author

Marie59 commented Mar 3, 2026

ohoh ! What do you think about this ? I think the function c.retrieve was still running even though the api key was none so I added a check to make sure the api key was not none

@bgruening bgruening merged commit 3c0ea4a into NordicESMhub:master Mar 3, 2026
11 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants