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

feat(Deployments): support ParameterOpenAPISchema #263

Merged
merged 8 commits into from
Oct 1, 2024

Conversation

mitchnielsen
Copy link
Contributor

Adds support for the ParameterOpenAPISchema for the Deployment resource.

Related to #238

Related to https://linear.app/prefect/issue/PLA-185/support-for-deployments-resource

Adds support for the ParameterOpenAPISchema for the Deployment resource.

Related to #238

Related to https://linear.app/prefect/issue/PLA-185/support-for-deployments-resource
@mitchnielsen
Copy link
Contributor Author

@felixpelletier - since you mentioned you were eager for this feature, do you mind taking a look here? Want to make sure this addresses real-world needs. Cheers!

@felixpelletier
Copy link
Contributor

@felixpelletier - since you mentioned you were eager for this feature, do you mind taking a look here? Want to make sure this addresses real-world needs. Cheers!

Sure thing!

I know it's an API limitation, and it's probably another team managing this part, but it's kind of limiting to have the parameter schemas field available for creation only. That means that any deployment that needs a change in the list of available parameters in the UI will need a new deployment or a replacement. If we simply replace the old deployment, then we lose any queued/running flow runs, and if we create a new one and switch over gradually, then it's not an ideal process (create the new deployment, release, move all schedules to the new deployment, release, remove old deployment after we're sure we don't need the historical logs, release).

We'll probably end up having a single parameter of type "object" named "parameters" and we will do all the parsing and validation ourselves (I believe someone else here already does this). At least this PR will give us the ability to do that.

I have two questions:

  • Could a change in this parameter trigger a resource replacement? I'm not sure that's the safest thing ever for everyone, but that may or may not be better than silently ignoring the change. Either way, the behavior should probably be documented somewhere.
  • Out of curiosity, why is parameter_openapi_schema not updatable in the API? Only 3 parameters aren't updatable (apart from the name and flow id, but that's a given): parameter_openapi_schema, pull_steps, disabled. I wonder why those 3 in particular.

Let me know if you have any questions. I'm happy to see some movement in here! The company here loves Terraform!

@mitchnielsen
Copy link
Contributor Author

mitchnielsen commented Sep 26, 2024

Out of curiosity, why is parameter_openapi_schema not updatable in the API?

That's a very fair question - let me reach out to the team and find out more and I'll follow up here.

Issue link (public)

Issue link (internal)

Could a change in this parameter trigger a resource replacement?

Great callout, yes this should definitely trigger a replacement. Will add that!

We don't update parameterOpenAPISchema because it's not supported by the
API, so we don't need to unmarshal the variable here.
@mitchnielsen mitchnielsen merged commit 5933437 into main Oct 1, 2024
@mitchnielsen mitchnielsen deleted the deployments-support-parameter-schema branch October 1, 2024 22:50
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants