search

AllenNeuralDynamics / aind-slims-api

AIND-specific access to data stored in SLIMS
MIT License
0 stars 1 forks source link

Configurable domain #48

Open mekhlakapoor opened 1 month ago

mekhlakapoor commented 1 month ago

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.

patricklatimer commented 1 month ago

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

mekhlakapoor commented 1 month ago

right now it's being hardcoded here: https://github.com/AllenNeuralDynamics/aind-slims-api/blob/73f8f76b187f5fcca6ed744024f5efc9e1acf73f/src/aind_slims_api/configuration.py#L13

patricklatimer commented 1 month ago

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.

mekhlakapoor commented 1 month ago

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.

patricklatimer commented 1 month ago

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!