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

Deployer types #117

Merged
merged 56 commits into from
Nov 1, 2023
Merged

Deployer types #117

merged 56 commits into from
Nov 1, 2023

Conversation

mfleader
Copy link
Member

Changes introduced with this PR

Adjust deployer based on parameters in workflow plugin.


By contributing to this repository, I agree to the contribution guidelines.

jaredoconnell and others added 30 commits August 4, 2023 10:16
Also improved debug messages
Mostly this is test cases, and some mild refactoring that should have a similar result
This ensures that the goroutines are done when the engine exits. This fixes a test case, and will be important when retrying is added to the deadlock check
This is a workaround for the situation where all of the steps are processing at the same time, all waiting for input
Also improved error logic
Required for prior commits to work
This adds the done message to the SDK
This means new error handling, so multiple errors can be reported
Force stop is good for when you do not need to wait for the ATP client, or something equivalant, to finish. Waiting for the ATP to finish means waiting for the steps to finish entirely.
I reduced the amount of code in one function, and reduced redundancy of unlocking
These ones were only shown on CI, not locally
@mfleader mfleader marked this pull request as ready for review November 1, 2023 14:02
@mfleader mfleader self-assigned this Nov 1, 2023
internal/step/dummy/provider_test.go Outdated Show resolved Hide resolved
internal/step/dummy/provider_test.go Outdated Show resolved Hide resolved
internal/step/dummy/provider_test.go Outdated Show resolved Hide resolved
internal/step/foreach/provider.go Outdated Show resolved Hide resolved
internal/step/lifecycle.go Outdated Show resolved Hide resolved
internal/step/plugin/provider.go Outdated Show resolved Hide resolved
internal/step/plugin/provider_test.go Outdated Show resolved Hide resolved
workflow/executor.go Outdated Show resolved Hide resolved
workflow/model.go Outdated Show resolved Hide resolved
workflow/workflow.go Outdated Show resolved Hide resolved
Copy link
Contributor

@jaredoconnell jaredoconnell left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The name/ID refactor made changes out of scope.
In addition, we should probably add a test for invalid deployer ID. I'll do that.

internal/step/plugin/provider.go Outdated Show resolved Hide resolved
internal/step/lifecycle.go Outdated Show resolved Hide resolved
internal/step/foreach/provider.go Outdated Show resolved Hide resolved
internal/step/dummy/provider_test.go Outdated Show resolved Hide resolved
internal/step/plugin/provider.go Outdated Show resolved Hide resolved
internal/yaml/parser.go Outdated Show resolved Hide resolved
workflow/executor.go Outdated Show resolved Hide resolved
workflow/model.go Outdated Show resolved Hide resolved
workflow/model.go Outdated Show resolved Hide resolved
workflow/model.go Outdated Show resolved Hide resolved
Matt Leader and others added 4 commits November 1, 2023 18:15
The docker deployer is already used extensively in the tests. No need to have a second deployer that also needs to be configured
Copy link
Contributor

@jaredoconnell jaredoconnell left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good to me. Tested locally.

@mfleader mfleader merged commit 9054fdf into main Nov 1, 2023
5 checks passed
@mfleader mfleader deleted the deployer-types branch November 1, 2023 22:57
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.

4 participants