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(deployment): add basic Deployment resource #202

Merged
merged 86 commits into from
Aug 15, 2024

Conversation

skyscrapr
Copy link
Contributor

@skyscrapr skyscrapr commented Jun 7, 2024

Summary

Adds Deployment resource with basic fields. More complete support for Deployments will come in future PRs (see #238).

Related to #238

Depends on #201.

Notes

  • API docs: https://app.prefect.cloud/api/docs#tag/Deployments

  • This PR is already big, so we're implementing some core fields here and the rest will be implemented in future PR(s)

  • CI Acceptance tests are sometimes failing with 503 service unavailable (and sometimes passing), but I tested the newly-added test by hand and it passed:

    $ go test ./... -count=1 -v -run TestAccResource_deployment
    ?       github.com/prefecthq/terraform-provider-prefect [no test files]
    .. etc ...
    ?       github.com/prefecthq/terraform-provider-prefect/internal/utils  [no test files]
    testing: warning: no tests to run
    PASS
    ok      github.com/prefecthq/terraform-provider-prefect/internal/provider/datasources   0.339s [no tests to run]
    === RUN   TestAccResource_deployment
    === PAUSE TestAccResource_deployment
    === CONT  TestAccResource_deployment
    --- PASS: TestAccResource_deployment (3.66s)
    PASS
    ok      github.com/prefecthq/terraform-provider-prefect/internal/provider/resources     4.252s
    

To do

  • Run a basic test to create and destroy a Flow + Deployment
  • Implement necessary attributes (others can come in a separate PR)
  • Ensure current acceptance tests pass
  • Expand acceptance tests as necessary

@skyscrapr skyscrapr requested review from gabcoyne and a team as code owners June 7, 2024 07:05
@parkedwards parkedwards added the enhancement New feature or request label Jun 11, 2024
@mitchnielsen mitchnielsen changed the title Resource deployment feat(flow): add Deployment resource Jun 12, 2024
@mitchnielsen
Copy link
Contributor

Thanks for the contribution @skyscrapr 🤝

Now that you've opened #201, could you please remove the Flow-related code to keep this PR focused on Deployments?

@skyscrapr
Copy link
Contributor Author

Deployments are dependent on flows
You cannot create a deployment without a flow_id.

I split flows into #201. But I'm unsure. How to create this PR if it requires another PR to be merged first. The branch will not pass tests.

If you explain how to do this, happy to update the PR and remove flows.

I have a third PR for deployment access that I haven't opened since it is depending on deployments and only makes the problem worse.

docs/resources/flow.md Show resolved Hide resolved
internal/api/deployments.go Show resolved Hide resolved
internal/provider/helpers/template.go Show resolved Hide resolved
Co-authored-by: Edward Park <[email protected]>
@mitchnielsen mitchnielsen changed the title feat(deployment): add Deployment resource feat(deployment): add basic Deployment resource Aug 15, 2024
@mitchnielsen mitchnielsen merged commit b44a018 into PrefectHQ:main Aug 15, 2024
7 checks passed
@mitchnielsen
Copy link
Contributor

Thanks for getting this one off the ground @skyscrapr 🤝

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

Successfully merging this pull request may close these issues.

4 participants