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

Make CI/CD Cloud Provider Test Conditional #2369

Merged

Conversation

tylergraff
Copy link
Contributor

Reference Issues or PRs

What does this implement/fix?

The cloud provider CI/CD test fails when run from forks of this repo which do not have credentials (and presumably, their own accounts) set up for the providers used by the test. This situation generates spurious CI/CD pipeline failure messages.

This PR adds the ability for repo fork maintainers to skip the CI/CD Cloud Provider Test by setting a repo variable 'NO_PROVIDER_CREDENTIALS'. If that variable is not set by the repo maintainer (the default case), the test behavior will be unchanged from before this PR.

Put a x in the boxes that apply

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds a feature)
  • Breaking change (fix or feature that would cause existing features not to work as expected)
  • Documentation Update
  • Code style update (formatting, renaming)
  • Refactoring (no functional changes, no API changes)
  • Build related changes
  • Other (please describe):

Testing

  • Did you test the pull request locally?
  • Did you add new tests?

Any other comments?

@marcelovilla
Copy link
Member

@tylergraff is there an existing issue for this we can reference? If not, would you mind creating one?

@tylergraff
Copy link
Contributor Author

@marcelovilla I just created this issue for reference:
refs #2379

Copy link
Contributor

@viniciusdc viniciusdc left a comment

Choose a reason for hiding this comment

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

LGTM, thanks for the contribution @tylergraff. I left a suggestion for rewriting the description a bit to also include the ref. issue, to help future contributions.

Co-authored-by: Vinicius D. Cerutti <51954708+viniciusdc@users.noreply.github.com>
Copy link
Contributor

@viniciusdc viniciusdc left a comment

Choose a reason for hiding this comment

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

Looks great!

@marcelovilla marcelovilla merged commit 147b8ce into nebari-dev:develop Apr 18, 2024
3 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Status: Done 💪🏾
Development

Successfully merging this pull request may close these issues.

None yet

3 participants