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

chore: add multirm module to ResourceManager #8857

Merged
merged 8 commits into from
Mar 4, 2024

Conversation

carolinaecalderon
Copy link
Contributor

@carolinaecalderon carolinaecalderon commented Feb 19, 2024

Description

Add a new multirm module and connect this to the master.
Add a new buildrm function to core.go where the rm is initialized.

Test Plan

See attached unit tests. In the future, I plan to write e2e tests too.

Checklist

  • Changes have been manually QA'd
  • User-facing API changes need the "User-facing API Change" label.
  • Release notes should be added as a separate file under docs/release-notes/.
    See Release Note for details.
  • Licenses should be included for new code which was copied and/or modified from any external code.

Ticket

RM-14, RM-38

@cla-bot cla-bot bot added the cla-signed label Feb 19, 2024
@carolinaecalderon carolinaecalderon force-pushed the chore-replace-rp-struct branch 3 times, most recently from e8fb827 to 7da5ba2 Compare February 19, 2024 22:23
@carolinaecalderon carolinaecalderon force-pushed the chore-replace-rp-struct branch 4 times, most recently from 283eac3 to 625a8eb Compare February 20, 2024 18:21
@carolinaecalderon carolinaecalderon force-pushed the carolinac/add-multirm-router-layer branch 2 times, most recently from ae9b6ff to 7f32d4f Compare February 20, 2024 20:06
@carolinaecalderon carolinaecalderon force-pushed the chore-replace-rp-struct branch 3 times, most recently from 2d2c07a to 481e835 Compare February 20, 2024 21:47
Copy link

codecov bot commented Feb 20, 2024

Codecov Report

Attention: Patch coverage is 76.76349% with 56 lines in your changes are missing coverage. Please review.

Project coverage is 47.29%. Comparing base (fa856ab) to head (0d058b4).

Additional details and impacted files
@@            Coverage Diff             @@
##             main    #8857      +/-   ##
==========================================
+ Coverage   47.22%   47.29%   +0.07%     
==========================================
  Files        1162     1162              
  Lines      175910   176109     +199     
  Branches     2235     2237       +2     
==========================================
+ Hits        83066    83284     +218     
+ Misses      92686    92667      -19     
  Partials      158      158              
Flag Coverage Δ
backend 42.41% <76.76%> (+0.32%) ⬆️
harness 63.94% <ø> (ø)
web 42.53% <ø> (ø)

Flags with carried forward coverage won't be shown. Click here to find out more.

Files Coverage Δ
master/internal/checkpoint_gc.go 68.04% <100.00%> (ø)
master/internal/config/resource_config.go 94.64% <ø> (ø)
master/internal/config/resource_manager_config.go 71.59% <ø> (ø)
master/internal/command/command_job_service.go 0.00% <0.00%> (ø)
master/internal/config/config.go 74.69% <90.32%> (+13.12%) ⬆️
master/internal/experiment_job_service.go 0.00% <0.00%> (ø)
master/internal/core.go 2.18% <0.00%> (-0.06%) ⬇️
master/internal/rm/multirm/multirm.go 86.18% <86.18%> (ø)

... and 4 files with indirect coverage changes

@carolinaecalderon carolinaecalderon force-pushed the chore-replace-rp-struct branch 2 times, most recently from f1f2ede to 6f3f00e Compare February 21, 2024 17:10
@carolinaecalderon carolinaecalderon force-pushed the carolinac/add-multirm-router-layer branch 2 times, most recently from 3878e37 to da24a75 Compare February 26, 2024 03:20
@carolinaecalderon carolinaecalderon changed the title chore: refactor master with DefaultRMRouter chore: add multirm module to ResourceManager Feb 26, 2024
@carolinaecalderon carolinaecalderon marked this pull request as ready for review February 26, 2024 19:14
@carolinaecalderon carolinaecalderon force-pushed the carolinac/add-multirm-router-layer branch from 83b539d to 9ce5b84 Compare March 4, 2024 16:53
@carolinaecalderon carolinaecalderon force-pushed the carolinac/add-multirm-router-layer branch from 4f12883 to 6d8b655 Compare March 4, 2024 17:55
@carolinaecalderon carolinaecalderon force-pushed the carolinac/add-multirm-router-layer branch from 6d8b655 to 0d058b4 Compare March 4, 2024 17:56
@carolinaecalderon carolinaecalderon merged commit c3012ff into main Mar 4, 2024
69 of 82 checks passed
@carolinaecalderon carolinaecalderon deleted the carolinac/add-multirm-router-layer branch March 4, 2024 19:15
carolinaecalderon added a commit that referenced this pull request Mar 5, 2024
carolinaecalderon added a commit that referenced this pull request Mar 6, 2024
This reverts commit c3012ff.

Revert "chore: refactor ResourceManager interface for multirm (#8847)"

This reverts commit 6857ecf.

Revert "chore: refactor proto, schema, and jobservice for multiRM (#8875)"

This reverts commit 7c6bec9.
carolinaecalderon added a commit that referenced this pull request Mar 6, 2024
This reverts commit c3012ff.

Revert "chore: refactor ResourceManager interface for multirm (#8847)"

This reverts commit 6857ecf.

Revert "chore: refactor proto, schema, and jobservice for multiRM (#8875)"

This reverts commit 7c6bec9.
carolinaecalderon added a commit that referenced this pull request Mar 6, 2024
This reverts commit c3012ff.

Revert "chore: refactor ResourceManager interface for multirm (#8847)"

This reverts commit 6857ecf.

Revert "chore: refactor proto, schema, and jobservice for multiRM (#8875)"

This reverts commit 7c6bec9.
carolinaecalderon added a commit that referenced this pull request Mar 7, 2024
* Revert "chore: add multirm module to ResourceManager (#8857)"

This reverts commit c3012ff.

Revert "chore: refactor ResourceManager interface for multirm (#8847)"

This reverts commit 6857ecf.

Revert "chore: refactor proto, schema, and jobservice for multiRM (#8875)"

This reverts commit 7c6bec9.
maxrussell pushed a commit that referenced this pull request Mar 21, 2024
* Revert "chore: add multirm module to ResourceManager (#8857)"

This reverts commit c3012ff.

Revert "chore: refactor ResourceManager interface for multirm (#8847)"

This reverts commit 6857ecf.

Revert "chore: refactor proto, schema, and jobservice for multiRM (#8875)"

This reverts commit 7c6bec9.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants