Skip to content

Conversation

@suhlig
Copy link
Collaborator

@suhlig suhlig commented May 17, 2020

This PR may be a bit on the heavy side, but the things it fixes are somehow related to each other. Let me know if you prefer separate PRs.

  1. Updates Python dependencies (letsencrypt => certbot, removes some unused ones)
  2. Removes ´--quietfromcf ssh` - it is probably broken in ibmcloud v1.1, and is not strictly required anyway.
  3. Improves the robustness of the certificate retrieval by re-trying in a loop. The first attempt is made without a sleep, so there should be no downside anymore.

@suhlig suhlig changed the title Update depencencies Update dependencies May 17, 2020
@lmsurpre lmsurpre self-requested a review July 20, 2020 23:15
Copy link
Member

@lmsurpre lmsurpre left a comment

Choose a reason for hiding this comment

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

It looks fine to me, but I no longer have a custom domain to test with. Any interest in taking over maintenance of this project?

@@ -1,4 +1,2 @@
cffi >= 0.8.0
letsencrypt >= 0.7.0
six >= 1.7
Copy link
Member

@lmsurpre lmsurpre Jul 21, 2020

Choose a reason for hiding this comment

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

this was for python2 compatibility and our goal was for it to run on either 2 or 3. however, I'm not even sure it was used and now python2 is end-of-life, so I'm ok with this change

suhlig and others added 7 commits April 11, 2021 12:31
letsencrypt is now certbot
The [`sleep(5)`](ibmjstart#31) had been bugging me. The failure induced by [`ibmcloud --quiet`](ibmjstart#32) triggered me to look for the exit code of the `cf ssh` command. Not the code keeps trying in a loop, but without an _initial_ sleep.

The sleep seems to be required in about 50% if the cases I tried.
@suhlig suhlig force-pushed the update-depencencies branch from 7ae505a to 13d57ea Compare April 11, 2021 10:55
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.

2 participants