-
-
Notifications
You must be signed in to change notification settings - Fork 1.8k
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
Support creation of Dependabot Organization and Repository Secrets #2874
Conversation
Codecov ReportAttention: Patch coverage is
❗ Your organization needs to install the Codecov GitHub app to enable full functionality. Additional details and impacted files@@ Coverage Diff @@
## main #2874 +/- ##
=======================================
Coverage ? 96.73%
=======================================
Files ? 147
Lines ? 14895
Branches ? 0
=======================================
Hits ? 14408
Misses ? 487
Partials ? 0 ☔ View full report in Codecov by Sentry. |
Hi @EnricoMi please can you re-review. I believe we've now addressed all your comments. |
tests/Organization.py
Outdated
def testOrgGetSecretAssertion(self): | ||
self.org = self.g.get_organization("demoorg") | ||
try: | ||
self.org.get_secret(secret_name="splat", secret_type="supersecret") | ||
except AssertionError: | ||
assert True |
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.
What exactly are you testing here?
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.
This test confirms that if you pass in a secret_type that isn't "dependabot" or "actions" it successfully throws an error.
I added this because codecov flagged this as untested
tests/Repository.py
Outdated
def testRepoGetSecretAssertion(self): | ||
repo = self.g.get_repo("demoorg/demo-repo-1") | ||
try: | ||
repo.get_secret(secret_name="splat", secret_type="supersecret") | ||
except AssertionError: | ||
assert True |
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.
Same here, what does this test assert?
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.
This test confirms that if you pass in a secret_type that isn't "dependabot" or "actions" it successfully throws an error.
I added this because codecov flagged this as untested
hi @EnricoMi is this ok to be merged now? |
* Moving tests to respective files * Correcting typo * Correcting typo * Creating ReplayData and Fixing tests, updating Organization.py to take secret_type on secrets call
Co-authored-by: Enrico Minack <github@enrico.minack.dev>
Co-authored-by: Enrico Minack <github@enrico.minack.dev>
Hello @EnricoMi I believe we've addressed all comments, can this be merged in? |
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.
Can you adopt this for all exception assertions, please?
Hi guys, thanks for working on this, we also would benefit a lot from adding a better support for Dependabot's secrets. 🙏 |
I think there is missing also the |
* Review Comments: Changing Testing of secret_type assertion * update delete secret option --------- Co-authored-by: Thomas Crowley <15927917+thomascrowley@users.noreply.github.com>
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.
LGTM!
Thanks for the approval @EnricoMi |
Inline with #2284, this change allows the creation and interactions with both "actions" and "dependabot" secrets both on the organization or a repository.
To achieve this I have added an additional parameter to all related methods to allow you to choose between "actions" or "dependabot" secrets. The default value is set to "actions" to help with back compatibility.