Skip to content

Conversation

gnalh
Copy link
Collaborator

@gnalh gnalh commented Mar 12, 2025

We shouldn't keep trying our endpoints if we've already detected an issue with the token / org combination.

Copy link

trunk-io bot commented Mar 12, 2025

Merging to main in this repository is managed by Trunk.

  • To merge this pull request, check the box to the left or comment /trunk merge below.

@gnalh gnalh changed the base branch from main to gabe/update-logging March 12, 2025 22:49
Copy link

trunk-staging-io bot commented Mar 12, 2025

Static BadgeStatic BadgeStatic BadgeStatic Badge

View Full Report ↗︎Docs

Copy link

trunk-io bot commented Mar 12, 2025

Static BadgeStatic BadgeStatic BadgeStatic Badge

View Full Report ↗︎Docs

@gnalh gnalh changed the base branch from gabe/update-logging to main March 27, 2025 20:33
@gnalh gnalh force-pushed the gabe/trunk-14336-bail-early-on-non-retry-errors-from-the-api branch from 58847ba to 9ff3222 Compare March 27, 2025 20:33
@gnalh gnalh marked this pull request as ready for review March 28, 2025 17:32
@gnalh gnalh requested a review from cmillar-trunk as a code owner March 28, 2025 17:32

// Verify only the quarantine config request was made
let requests = state.requests.lock().unwrap().clone();
assert_eq!(requests.len(), 0);
Copy link
Collaborator

Choose a reason for hiding this comment

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

Why are we expecting 0 requests instead of 1?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

The request didn't go through because it raises an error above.

Copy link
Collaborator

Choose a reason for hiding this comment

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

But how do you get the error from the state server without sending a request to it? Doesn't the cli have to send a request to get the 404 from the server?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

The logic in the mock_server is such that if there's an error during the request then this will not be populated if there is an error. I agree it's confusing given the name and we should move towards it always being populated.

@gnalh gnalh requested a review from cmillar-trunk March 28, 2025 18:02
@gnalh gnalh force-pushed the gabe/trunk-14336-bail-early-on-non-retry-errors-from-the-api branch from 0bd87fb to 6452ff6 Compare April 1, 2025 20:44
@gnalh gnalh marked this pull request as draft April 11, 2025 05:41
@gnalh
Copy link
Collaborator Author

gnalh commented May 20, 2025

Closing this since it isn't as easy as we'd hoped

@gnalh gnalh closed this May 20, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

2 participants