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.

Sign up for GitHub

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

Jump to bottom

Refactor cert to create and wait in the background #11321

Merged
merged 2 commits into from
Jul 14, 2022
Merged

Refactor cert to create and wait in the background #11321

merged 2 commits into from
Jul 14, 2022

Conversation

liam-j-bennett
Copy link
Contributor

@liam-j-bennett liam-j-bennett commented Jul 12, 2022 •
edited by ArthurSens

Description

Refactor the way we create and wait for certs.

These changes should lead to:

  • Less failures, as we retry and wait 5 times
  • Earlier failures, as we fail out before we start deploying things

Release Notes

NONE

Werft options:

  • /werft with-preview

@liam-j-bennett liam-j-bennett requested a review from a team July 12, 2022 13:47
@liam-j-bennett liam-j-bennett force-pushed the ljb/refactor-cert-creation-and-wait branch from bb7c35d to 548f40d Compare July 12, 2022 13:48
@liam-j-bennett
Copy link
Contributor Author

I've removed some of the werft.log etc. from the previous code. Mainly, as I'm not sure how it will work/look in practice.

@liam-j-bennett liam-j-bennett force-pushed the ljb/refactor-cert-creation-and-wait branch from 548f40d to f42d0fa Compare July 13, 2022 11:07
@liam-j-bennett liam-j-bennett marked this pull request as ready for review July 13, 2022 11:07
@werft-gitpod-dev-com
Copy link

started the job as gitpod-build-ljb-refactor-cert-creation-and-wait.3 because the annotations in the pull request description changed
(with .werft/ from main)

ArthurSens
ArthurSens reviewed Jul 13, 2022
View reviewed changes
Copy link
Contributor

@ArthurSens ArthurSens left a comment

Choose a reason for hiding this comment

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

👋

I'm not really proficient with Typescript, so I think it would be better if Mads could give a proper review here. Reading through the changes there was only one thing that looked off to me 🤔

mads-hartmann
mads-hartmann approved these changes Jul 14, 2022
View reviewed changes
Copy link
Contributor

@mads-hartmann mads-hartmann left a comment

Choose a reason for hiding this comment

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

I left a few minor comments about changes I'd like to see, but they can be done in a follow up PR if you like so I'll approve ☺️ I have added the hold label in case you want to take care of them in this PR.

.werft/jobs/build/deploy-to-preview-environment.ts Outdated Show resolved Hide resolved
.werft/util/certs.ts Outdated Show resolved Hide resolved
.werft/util/certs.ts Outdated Show resolved Hide resolved
.werft/util/certs.ts Outdated Show resolved Hide resolved
.werft/util/certs.ts Outdated Show resolved Hide resolved
@roboquat roboquat merged commit bb5954a into main Jul 14, 2022
@roboquat roboquat deleted the ljb/refactor-cert-creation-and-wait branch July 14, 2022 09:32
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@mads-hartmann mads-hartmann mads-hartmann approved these changes

@ArthurSens ArthurSens ArthurSens left review comments

Assignees
No one assigned
Labels
release-note-none size/L team: devx
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
4 participants
@liam-j-bennett @mads-hartmann @ArthurSens @roboquat