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

[Feature Request] Externalise the environment.yaml #2516

Closed
LittleColin opened this issue Mar 27, 2024 · 3 comments
Closed

[Feature Request] Externalise the environment.yaml #2516

LittleColin opened this issue Mar 27, 2024 · 3 comments
Assignees
Labels
enhancement New feature or request

Comments

@LittleColin
Copy link

LittleColin commented Mar 27, 2024

Is your feature request related to a problem? Please describe.
We aim to use a build-once approach for Docker containers so that what we test in one environment is promoted to the next without change. The pf flow build command generates a Dockerfile that is environment specific, effectively hardcoding the URL to the openai endpoint inside the container.

Describe the solution you'd like
Allow all of the connections in connections directory to be specified via environment variables, like the api key is. At least allowing api_base to use environment vars.

Describe alternatives you've considered
Customising the Dockerfile ourselves and moving away from pf flow build or adding in a step after pf flow build and before building the Docker container to update the connections files with api_base: ${env:OUR_CONNECTION_API_BASE}

Additional context
Add any other context or screenshots about the feature request here.

@LittleColin LittleColin added the enhancement New feature or request label Mar 27, 2024
@elliotzh
Copy link
Member

elliotzh commented Mar 28, 2024

Hi @LittleColin thank you for reaching out to us. Let me confirm - your request is:

for now, we will extract secrets (api_key in most cases) and replace them with environment variables only; however, you want us to provide a parameter to customize the fields to be replaced with environment variables.

We will discuss this feature ask first. On the other hand, we haven't done that support before because the configuration can be complicated to specify fields to be replaced, especially given different type of connections may have different fields.

Besides your workaround to add a post-process step:

  1. we have a feature that we will respect environment variable of a specific name with a higher priority, then the value in connection yaml
    1. for example, if the connection name is my-connection, you can set an environment variable named MY_CONNECTION_API_BASE and we will respect it on serving.
    2. sorry we haven't mentioned this in current doc, but we have a test for this: link
  2. you can also use a CustomConnection and mark all fields you want to replace as secrets - we're hiding all secrets instead of some specific fields.

@LittleColin
Copy link
Author

Thanks for the response, yes the override appears to work and addresses the feature request. However, the logging could be improved in this area as the myconnection.yaml content is displayed including the api base URL but with no indication that it has been overridden by an environment var. Also, if there's a connection issue, the error does not show the api base that was being connected to (I cannot see anywhere in the logs that the actual api base is emitted).

Suggest changing the cat of myconnection.yaml to indicate value has been overridden by an env var as would be good to know even on a successful connection.

@elliotzh
Copy link
Member

Thank you for your suggestion. We will use an internal task to track the log improvement and close this issue first.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

No branches or pull requests

2 participants