-
Notifications
You must be signed in to change notification settings - Fork 17
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(automations): datasource support #310
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
looking good from my side, great stuff. weird that the CI checks didn't run?
|
||
//nolint:paralleltest // we use the resource.ParallelTest helper instead | ||
func TestAccDatasource_automation(t *testing.T) { | ||
eventTriggerAutomationResourceName := testutils.NewRandomPrefixedString() |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
(no changes required) something I forgot to mention in the Resource PR: because we're using ephemeral workspaces, I don't believe we need the resource names to be random
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
hmm - i thought they had to be unique (even across test cases) so that they wouldn't overwrite the resource in the TF state. i know this is the datasource test, but if we were in the resource test and used the same names, wouldn't they perform an Update()?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Fair, I may be writing tests slightly with different expectations - I assume each test case is updating an existing resource so I can test Update
functionality. Sounds like it may just be a different use case?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ah interesting
resolves https://linear.app/prefect/issue/PLA-577/tfp-automations-scaffold-datasource
followup to #305