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.

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

[Backport - 2.8] Garbage Collect Determined Pipeline Users (#9491) [CORE-2131] #9658

Merged
merged 2 commits into from Jan 30, 2024

Conversation

acohen4
Copy link
Contributor

@acohen4 acohen4 commented Jan 16, 2024

No description provided.

Copy link

codecov bot commented Jan 25, 2024

Codecov Report

Attention: 103 lines in your changes are missing coverage. Please review.

Comparison is base (62fd3c1) 49.88% compared to head (5eebd73) 49.83%.

Files Patch % Lines
src/server/pps/server/determined.go 9.25% 98 Missing ⚠️
src/server/pps/server/api_server.go 0.00% 5 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##            2.8.x    #9658      +/-   ##
==========================================
- Coverage   49.88%   49.83%   -0.05%     
==========================================
  Files         565      565              
  Lines       69162    69252      +90     
==========================================
+ Hits        34500    34515      +15     
- Misses      31226    31307      +81     
+ Partials     3436     3430       -6     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

Copy link
Contributor

@robert-uhl robert-uhl left a comment

Choose a reason for hiding this comment

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

I do think it’s worth considering how the selector should work, even though this has already gone into master (where it can still be changed …). But this all LGTM.

"suite": "pachyderm",
"pipelineName": pipeline.Name,
"project": pipeline.GetProject().String(),
"determined": "true",
Copy link
Contributor

Choose a reason for hiding this comment

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

Rather than determined=true, I wonder if this should be type=determined or similar.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

great suggestion <3. I'll open a ticket for this and handle it in a separate PR

@acohen4 acohen4 merged commit 9cdaf49 into 2.8.x Jan 30, 2024
25 checks passed
@acohen4 acohen4 changed the title [Backport - 2.8] Garbage Collect Determined Pipeline Users (#9491) [Backport - 2.8] Garbage Collect Determined Pipeline Users (#9491) [CORE-2131] Jan 30, 2024
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.

None yet

3 participants