Skip to content
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.

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

Implement profiles to specify scicat instances more easily #275

Merged
merged 4 commits into from
Mar 26, 2025
Merged

Conversation

jl-wynen
Copy link
Collaborator

@jl-wynen jl-wynen commented Mar 4, 2025

Fixes #37

How to use profiles

With this PR, we can connect to ESS SciCat using

client = Client.from_token("ess", token=<TOKEN>)

However, this will only work on a non-ess machine if the user has their SSH agent set up properly. If they don't, they still need to provide a file transfer manually. E.g.:

transfer = SFTPFileTransfer(
    host="login.esss.dk",
    username=<USERNAME>,
    password=<PASSWORD>,
)
client = Client.from_token("ess", token=<TOKEN>, file_transfer=transfer)

I am happy to add profiles for other instances to the package if requested!

Loading profiles from files

This PR includes the option of loading profiles from TOML files. But this is very limited at the moment and cannot load file transfers. This is because I am unsure how useful this actually is. So reviewer, please let me know if you think that this is valuable enough to expand support. Or if it should be removed because it is too complicated.

@jl-wynen jl-wynen requested review from nitrosx and YooSunYoung March 4, 2025 12:48
@jl-wynen
Copy link
Collaborator Author

There is an open question: How an a user authenticate with SFTP if they don't use an SSH agent? The file transfer allows passing username and password. But the current profile implementation does not support that. I think this is required for many people. But it is tricky because the scicat credentials can be different than the sftp credentials. Otherwise, we could forward arguments from Client.from_credentials.

@jl-wynen jl-wynen marked this pull request as draft March 25, 2025 12:53
@jl-wynen jl-wynen marked this pull request as ready for review March 26, 2025 13:07
@jl-wynen
Copy link
Collaborator Author

Let's postpone passing credentials to the file transfer to later. This is a larger issue that should not block merging this PR.

@jl-wynen jl-wynen enabled auto-merge March 26, 2025 13:08
@jl-wynen jl-wynen merged commit 04f951a into main Mar 26, 2025
12 checks passed
@jl-wynen jl-wynen deleted the profiles branch March 26, 2025 13:11
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.

SciCat profiles
2 participants