-
Notifications
You must be signed in to change notification settings - Fork 1
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
Configurable domain #48
Comments
You should be able to pass the url (and username/password) upon instantiating the client, or set it via environment variables (SLIMS_URL). Dynamic foraging folks use the environment variables currently |
right now it's being hardcoded here: |
Since it's a Pydantic BaseSettings, environment variables will override the default if they're set, which is pretty cool, though I have mixed feelings about overusing environment variables for things like production rig software as they're another thing to keep track of. The client also accepts optional keyword arguments to set the URL, username, and password. In recent SIPE rig software, we've been using a centralized password manager that apps can query for relevant credentials like this then pass on instantiation and that has been working pretty well. Real easy to roll out changes. |
I guess that's fair. We can close this issue if you'd like, but I'm still hesitant to be relying on default hardcoded endpoints. |
I see what you're saying now, and yeah I wouldn't be opposed to removing it so as to discourage relying on the hardcoded default and encourage users to be intentional about the url they're using. Also because during development we want people to be switching between the dev and prod instances. Thanks for looking out! |
Is your feature request related to a problem? Please describe.
The aind-slims-api should be useable for both dev and prod slims deployments. Instead of hard-coding the domain, let's have it as a configurable input for the SlimsClient
Describe the solution you'd like
A clear and concise description of what you want to happen.
Describe alternatives you've considered
A clear and concise description of any alternative solutions or features you've considered.
Additional context
Add any other context or screenshots about the feature request here.
The text was updated successfully, but these errors were encountered: