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

feat: add model version streaming #9029

Merged
merged 29 commits into from
Apr 4, 2024
Merged

Conversation

gt2345
Copy link
Contributor

@gt2345 gt2345 commented Mar 20, 2024

Description

Add model version streaming, this is not yet exposed to the WebUI.
There will be a separate PR for EE to handle rbac.

Test Plan

Creating/editing/deleting model and model version work as expected.
Connect to ws://localhost:8080/stream, should be able to subscribe and unsubscribe for modelversions.
Sample message:

{
    "Sync_ID": "5vdsfsd",
    "Known": {
        "Projects": "63",
        "models": ""
    },
    "Subscribe": {
        "projects": {
            "project_ids": [13],
            "since": 0
        },
        "models": {
            "model_ids": [11],
            "user_ids": [1],
            "model_names": ["eee"]
        },
        "modelversions": {
            "model_ids": [15]
        }
    } 
}

Commentary (optional)

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

MD-262

Copy link

netlify bot commented Mar 20, 2024

Deploy Preview for determined-ui ready!

Name Link
🔨 Latest commit ecff7d3
🔍 Latest deploy log https://app.netlify.com/sites/determined-ui/deploys/660f150b0000bd0008718b01
😎 Deploy Preview https://deploy-preview-9029--determined-ui.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site configuration.

@gt2345 gt2345 marked this pull request as draft March 20, 2024 19:36
@gt2345 gt2345 changed the title Gt/262 model version streaming feat: add model version streaming Mar 21, 2024
Copy link

codecov bot commented Mar 21, 2024

Codecov Report

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

Project coverage is 53.23%. Comparing base (2c6fec7) to head (ecff7d3).

Additional details and impacted files
@@            Coverage Diff             @@
##             main    #9029      +/-   ##
==========================================
+ Coverage   46.16%   53.23%   +7.06%     
==========================================
  Files        1171      770     -401     
  Lines      143382    82363   -61019     
  Branches     2410        0    -2410     
==========================================
- Hits        66196    43843   -22353     
+ Misses      76981    38520   -38461     
+ Partials      205        0     -205     
Flag Coverage Δ
backend 43.37% <72.56%> (+0.02%) ⬆️
harness 63.99% <38.46%> (+1.64%) ⬆️
web ?

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

Files Coverage Δ
master/internal/stream/authz_basic_impl.go 87.50% <100.00%> (+4.16%) ⬆️
master/internal/stream/messages.go 0.00% <ø> (ø)
master/internal/stream/models.go 63.54% <100.00%> (ø)
master/internal/stream/projects.go 77.01% <100.00%> (ø)
master/internal/stream/subscription.go 96.00% <100.00%> (+1.17%) ⬆️
master/internal/stream/test_util.go 70.66% <100.00%> (+2.09%) ⬆️
master/internal/stream/util.go 69.49% <100.00%> (ø)
master/internal/stream/publisher.go 66.53% <95.00%> (+1.44%) ⬆️
master/cmd/stream-gen/main.go 0.00% <0.00%> (ø)
harness/determined/common/streams/_client.py 78.72% <60.00%> (-1.28%) ⬇️
... and 2 more

... and 415 files with indirect coverage changes

@gt2345 gt2345 marked this pull request as ready for review March 22, 2024 16:00
@gt2345 gt2345 force-pushed the gt/262-model-version-streaming branch from b7b5c63 to 999465d Compare March 26, 2024 16:40
Copy link
Contributor

@ashtonG ashtonG left a comment

Choose a reason for hiding this comment

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

frontend code ok!

Copy link
Contributor

@corban-beaird corban-beaird left a comment

Choose a reason for hiding this comment

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

Looks great! 🚀

@gt2345 gt2345 force-pushed the gt/262-model-version-streaming branch from 0d19791 to ceab565 Compare April 3, 2024 20:58
Copy link
Contributor

@azhou-determined azhou-determined left a comment

Choose a reason for hiding this comment

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

lgtm

@gt2345 gt2345 merged commit 9dce6f0 into main Apr 4, 2024
69 of 82 checks passed
@gt2345 gt2345 deleted the gt/262-model-version-streaming branch April 4, 2024 21:43
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