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

Adding more features support in SSL certificates #15

Merged
merged 3 commits into from
Dec 21, 2020

Conversation

trolldbois
Copy link
Contributor

  • Support parameters in "Listing SSL Certificates" to allow searching for specific certificates
  • Add support for "Get SSL Certificate" by cert sslId

@trolldbois trolldbois requested a review from a team as a code owner April 10, 2020 13:52
lukwam
lukwam previously approved these changes Aug 20, 2020
@coreone coreone added the enhancement New feature or request label Sep 24, 2020
@coreone
Copy link
Member

coreone commented Sep 30, 2020

@trolldbois sorry it took a while to circle back to this. Can you rebase on current master as there are conflicts here now. We are also dropping Python 2.7 support, so that should clear up some of the tests after the rebase as well.

@ravanapel
Copy link
Contributor

Any update on this? Anything I can help with? This functionality would be useful for me.

@rhlin888
Copy link

This functionality would be useful for me as well. Would also like to know if there are any updates.

@trolldbois
Copy link
Contributor Author

@coreone

@coreone
Copy link
Member

coreone commented Nov 12, 2020

There were a couple comments here that didn't seem to get addressed. Additionally, we have also transitioned to the main branch now, so if you could rebase against that base it should pick up the new GitHub Actions.

@coreone coreone changed the base branch from master to main November 17, 2020 14:27
@coreone coreone dismissed lukwam’s stale review November 17, 2020 14:27

The base branch was changed.

@coreone
Copy link
Member

coreone commented Nov 17, 2020

@trolldbois I updated the base branch to main. Please rebase on that branch.

@coreone
Copy link
Member

coreone commented Dec 17, 2020

@trolldbois I will approve/merge these changes if you can rebase to resolve the conflict and the remaining comments

@codecov
Copy link

codecov bot commented Dec 21, 2020

Codecov Report

Merging #15 (086005b) into main (da65612) will decrease coverage by 2.74%.
The diff coverage is 45.83%.

Impacted file tree graph

@@            Coverage Diff             @@
##             main      #15      +/-   ##
==========================================
- Coverage   95.38%   92.64%   -2.75%     
==========================================
  Files          11       11              
  Lines         412      435      +23     
==========================================
+ Hits          393      403      +10     
- Misses         19       32      +13     
Flag Coverage Δ
unittests 92.64% <45.83%> (-2.75%) ⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

Impacted Files Coverage Δ
cert_manager/_helpers.py 68.23% <27.27%> (-6.09%) ⬇️
cert_manager/ssl.py 73.68% <61.53%> (-26.32%) ⬇️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update da65612...086005b. Read the comment docs.


@paginate
def list(self, **kwargs):
# size, position, commonName, subjectAlternativeName, status , sslTypeId
Copy link
Member

Choose a reason for hiding this comment

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

Probably replace all these comments with a link to the doc:

https://sectigo.com/uploads/audio/Certificate-Manager-20.1-Rest-API.html#resource-SSL-list

from requests.exceptions import HTTPError

from ._helpers import Pending
from ._endpoint import Endpoint
Copy link
Member

Choose a reason for hiding this comment

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

Only the from ._helpers import paginate import is needed here, right?

@coreone coreone merged commit 20beaee into broadinstitute:main Dec 21, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants