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

fix: Support serialized JSON environment variables #1415

Merged
merged 1 commit into from
Feb 24, 2024

Conversation

baumandm
Copy link
Contributor

Some of the newer AI configs expect a dict, with the assumption that a custom querybook_config.yaml file is mounted into the Docker volume. Unfortunately that's not straightforward in our deployment, so we're exclusively using environment variables. As a workaround we've hardcoded the dict type configs into our Docker image, which is less flexible.

To address this, this PR adds support for deserializing JSON dicts/lists in configs. It handles deserializing for both environment variables and YAML configs, so it should be backwards compatible with existing config files.

An example config:

AI_ASSISTANT_CONFIG='{ "default": { "model_args": { "model_name": "gpt-3.5-turbo", "temperature": 0.2 }}}'

Two configs already expected serialized JSON: FLASK_CACHE_CONFIG and GOOGLE_CREDS. Now they can be specified as either string-serialized JSON (like before) or a dict.

Note: I restricted this to dict and list types to avoid issues with int or bool types, e.g. where Querybook expects a value of "true" not true. I didn't want to go through the entire code base to update all of those.

@jczhong84 jczhong84 merged commit c3be536 into pinterest:master Feb 24, 2024
3 checks passed
@baumandm baumandm deleted the external/json-configs branch February 28, 2024 14:46
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.

2 participants