Skip to content
This repository was archived by the owner on Jan 21, 2025. It is now read-only.

Added more logging + increased delay after creating a fresh branch#168

Merged
mordamax merged 4 commits intomasterfrom
mak/166-validation-error
Mar 2, 2023
Merged

Added more logging + increased delay after creating a fresh branch#168
mordamax merged 4 commits intomasterfrom
mak/166-validation-error

Conversation

@mordamax
Copy link
Copy Markdown
Contributor

@mordamax mordamax commented Mar 1, 2023

Aims to close #166

@mordamax mordamax requested a review from a team as a code owner March 1, 2023 22:39
Bullrich
Bullrich approved these changes Mar 2, 2023
View reviewed changes
Copy link
Copy Markdown
Contributor

@Bullrich Bullrich left a comment

Choose a reason for hiding this comment

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

Seems sensible!

src/gitlab.ts
let wasBranchRegistered = false;
const waitForBranchMaxTries = 3;
const waitForBranchRetryDelay = 1024;
const waitForBranchRetryDelayMs = 2000;
Copy link
Copy Markdown
Contributor

@Bullrich Bullrich Mar 2, 2023

Choose a reason for hiding this comment

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

I'm mentally bothered that it is 2000 and no 2048 haha

But yeah, it's two seconds so it's a rounder number

mutantcornholio
mutantcornholio suggested changes Mar 2, 2023
View reviewed changes
Copy link
Copy Markdown
Contributor

@mutantcornholio mutantcornholio left a comment

Choose a reason for hiding this comment

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

This PR does throwing errors and logging errors at the same time, which results in double logging, or logging successfully handled errors.
Can we change it the way that we'll log the errors when handling those?

@mordamax
Copy link
Copy Markdown
Contributor Author

mordamax commented Mar 2, 2023

This PR does throwing errors and logging errors at the same time, which results in double logging, or logging successfully handled errors. Can we change it the way that we'll log the errors when handling those?

good point, rolled back and added error log on the common handler.
So before in any case (error or ok) it was logging just debug, and it wasn't clear if there's an error happened. Now this will be noticeable

Thanks

@mordamax mordamax requested a review from mutantcornholio March 2, 2023 12:39
@mordamax mordamax merged commit f9db80b into master Mar 2, 2023
@mordamax mordamax deleted the mak/166-validation-error branch March 2, 2023 15:10
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Reviewers

2 more reviewers

@mutantcornholio mutantcornholio mutantcornholio approved these changes

@Bullrich Bullrich Bullrich approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Bug: sometimes gitlab api fails with ValidationError: "id" is required

3 participants

@mordamax @mutantcornholio @Bullrich