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

QA Kubernetes Tasks #1559

Merged
merged 8 commits into from
Sep 27, 2019
Merged

QA Kubernetes Tasks #1559

merged 8 commits into from
Sep 27, 2019

Conversation

joshmeek
Copy link

@joshmeek joshmeek commented Sep 27, 2019

Thanks for contributing to Prefect!

Please describe your work and make sure your PR:

  • adds new tests (if appropriate)
  • updates CHANGELOG.md (if appropriate)
  • updates docstrings for any new functions or function arguments, including docs/outline.toml for API reference docs (if appropriate)

Note that your PR will not be reviewed unless all three boxes are checked.

What does this PR change?

QA'd the k8s tasks and found that the API secret was required, adjusted so it can be provided as None. This required tests to be updates slightly. Also added tests for each task's in/out of cluster configuration connections.

Why is this PR important?

Secret was required and some connection options went untested

@joshmeek joshmeek added the integrations Related to integrations with other services label Sep 27, 2019
@codecov
Copy link

codecov bot commented Sep 27, 2019

Codecov Report

Merging #1559 into master will increase coverage by 1.68%.
The diff coverage is 99%.

cicdw
cicdw previously approved these changes Sep 27, 2019
Copy link
Member

@cicdw cicdw left a comment

Choose a reason for hiding this comment

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

Awesome nice QA work; not going to Request Changes since I know this is touches a large number of tasks, but should we update the documentation anywhere to describe this pattern of setting the secret key to None to enable a different connection option?

@joshmeek
Copy link
Author

Yeah I’ll update the docstrings, good call

@joshmeek joshmeek merged commit cf5cf85 into master Sep 27, 2019
@joshmeek joshmeek deleted the k8s_tests branch September 27, 2019 18:19
zanieb added a commit that referenced this pull request Apr 13, 2022
…-flowrunstates

Prevent transitions out of terminal states for flow run states
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
integrations Related to integrations with other services
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants