-
Notifications
You must be signed in to change notification settings - Fork 349
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
fix: ensure number of project keys possible for testing is not exceeded #9501
Conversation
✅ Deploy Preview for determined-ui canceled.
|
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #9501 +/- ##
=======================================
Coverage 48.99% 48.99%
=======================================
Files 1235 1235
Lines 160191 160191
Branches 2780 2780
=======================================
+ Hits 78482 78483 +1
+ Misses 81534 81533 -1
Partials 175 175
Flags with carried forward coverage won't be shown. Click here to find out more. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
looks good! Just one minor question
return err | ||
numRequests := 5 | ||
errgrp := errgroupx.WithContext(ctx) | ||
for i := 0; i < numRequests; i++ { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
400 vs 5 is a huge difference...I doubt there will be so many concurrent keygens at once, but was there a requirement to handle any specific number of concurrent keygen attempts?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Originally, I had wrote this to be a stress test to ensure that we could handle the case when we start to exhaust the set of project keys for a given project name pattern, but realistically I don't imagine we'll have folks creating 20 projects concurrently 20 times. It being able to support the creation of 5 project synchronously without issue is sufficient for the cadence we expect projects to be created.
Ticket
Description
This reduces the number of requests being made by the
TestConcurrentProjectKeyGenerationAttempts
test to ensure we do not exceed the number of possible project keys given the project name, which was resulting in this test flaking.Test Plan
TestConcurrentProjectKeyGenerationAttempts
passesChecklist
docs/release-notes/
.See Release Note for details.