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.

Sign up for GitHub

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

Jump to bottom

Have AlchemiscaleClient automatically pull settings from the environment #355

Open
jthorton opened this issue Feb 5, 2025 · 3 comments · May be fixed by #368
Open

Have AlchemiscaleClient automatically pull settings from the environment #355

jthorton opened this issue Feb 5, 2025 · 3 comments · May be fixed by #368
Labels
component-compute-client component-user-client
Milestone
Release 0.6.1 - i...

Comments

@jthorton
Copy link

jthorton commented Feb 5, 2025

I currently store my alchemiscale key, identifier and url in environment variables and find that I often have to write the same bit of code to get them from the environment and create the client object like so

import os
from alchemiscale import AlchemiscaleClient
client = AlchemiscaleClient(api_url=os.environ["ALCHEMISCALE_URL"], key=os.environ["ALCHEMISCALE_KEY"], identifier=os.environ["ALCHEMISCALE_ID"])

however it would be great if we could have the client do this automatically if the variables are present. We currently do this in asap-alchemy which helps when building a CLI which interacts with alchemiscale. Porting this to alchemiscale would be great and save packages from reimplementing this.

In broad terms, what are you trying to do?

Briefly detail what you have already tried

Describe the new feature(s) that would help you to achieve this
@mikemhenry
Copy link
Collaborator

Is your proposal that

client = AlchemiscaleClient()

Would just work if those environment variables exist?

@dotsdl
Copy link
Member

dotsdl commented Feb 12, 2025

Thanks @jthorton! I've added this to the 0.6.1 milestone!

@LilDojd
Copy link
Contributor

LilDojd commented Feb 21, 2025

Hi! Wanted to clarify something before I address this issue:

AFAIC when both an environment variable and an explicit argument are provided, the argument should take precedence. In such cases, we should issue warnings to notify the user of stale environment variable.

import os
from alchemiscale import AlchemiscaleClient
client = AlchemiscaleClient(api_url="someurl", key="somekey", identifier="someid")

# stderr
.../client.py:69: UserWarning: Environment variable ALCHEMISCALE_URL is set to "someotherurl", but an explicit URL ("someurl") was provided. Using the explicit URL.
.../client.py: UserWarning: Environment variable ALCHEMISCALE_KEY is set, but an explicit key argument was provided. Using the explicit key.
.../client.py: UserWarning: Environment variable ALCHEMISCALE_ID is set to "someotherid", but an explicit identifier ("someid") was provided. Using the explicit identifier.

What do you think?

LilDojd pushed a commit to LilDojd/alchemiscale that referenced this issue Feb 21, 2025
feat: add environment variable support for client initialization params
2144d7d 
The commit adds support for initializing AlchemiscaleClient with
environment variables, allowing configuration via ALCHEMISCALE_URL,
ALCHEMISCALE_ID, and ALCHEMISCALE_KEY

Adds test for this behavior

Closes OpenFreeEnergy#355
@LilDojd LilDojd linked a pull request Feb 21, 2025 that will close this issue
Draft
LilDojd added a commit to LilDojd/alchemiscale that referenced this issue Feb 21, 2025
@LilDojd
feat: add environment variable support for client initialization params
63a2635 
The commit adds support for initializing AlchemiscaleClient with
environment variables, allowing configuration via ALCHEMISCALE_URL,
ALCHEMISCALE_ID, and ALCHEMISCALE_KEY

Adds test for this behavior

Closes OpenFreeEnergy#355
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
component-compute-client component-user-client
Projects
None yet
Milestone
Release 0.6.1 - incremental release
Development

Successfully merging a pull request may close this issue.

feat: add environment variable support for client initialization params LilDojd/alchemiscale
4 participants
@dotsdl @mikemhenry @jthorton @LilDojd