feat(frontend): add model file upload and version creation - #8078
feat(frontend): add model file upload and version creation#8078tanishqgandhi1908 wants to merge 6 commits into
Conversation
Automated Reviewer SuggestionsBased on the
|
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## main #8078 +/- ##
============================================
+ Coverage 93.52% 93.74% +0.22%
+ Complexity 4709 4707 -2
============================================
Files 1195 1191 -4
Lines 48466 48255 -211
Branches 5378 5376 -2
============================================
- Hits 45326 45237 -89
+ Misses 1694 1588 -106
+ Partials 1446 1430 -16
*This pull request uses carry forward flags. Click here to find out more. ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
0564f23 to
fb2ad19
Compare
…ection The card describes the newest version whatever the version picker shows, but its file name and size were written inside onVersionSelected, guarded on the selected version happening to be versions[0]. Renaming while browsing an older version therefore left "Latest version file" on a path built from the old name. Fetch those facts in their own right instead, mirroring the dataset page's retrieveLatestVersionFile, and refresh them alongside the tree after a rename. Using retrieveModelVersionList would also have fixed the display, but it resets the picker to the newest version, which is not something a rename should do. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
- Drop the invented .progress-header flex rule. The class is unstyled on main, so adding it silently changed the dataset upload row's layout, against this PR's claim of leaving dataset behavior alone. - Stop listing the newest version's objects twice on every model page load. onVersionSelected already fetches that tree whenever the newest version is the one on screen, so the card now reuses that response and the extra fetch runs only when the two differ — after a rename while browsing an older one. - Reword the conflicting-file modal, which said "in this dataset" and is now shown to model owners too. - Remove a duplicate VersionUploaderComponent import and a comment orphaned when its rule moved to the shared panel. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
… rename - Port the three loadUploadSettings fallback tests that were dropped when the panel moved out of DatasetDetailComponent. They pin the invariant the code comment warns about: a failed or unparsable settings fetch must leave the fields alone, because a NaN concurrency limit stalls the queue outright (`activeUploads < NaN` is never true). - Renaming a model reopens the file that was on screen instead of resetting to the version's first. Only the paths change, so the file is still there. - Derive the admin upload card's title from FileResourceEndpoint.label, so a resource family is named once rather than in two places that could drift. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
| // Settings | ||
| // =========================================================================== | ||
|
|
||
| onSaveModelName(): void { |
There was a problem hiding this comment.
A rename here invalidates any upload that is still in flight.
multipartUpload() takes resourceName as an argument, so it captured the old name at start. Once this updates modelName, the remaining part and finish calls keep sending the old name and the backend resolves them through getModelBy(ownerEmail, modelName) -> 404. Aborting doesn't rescue it either: onClickAbortUploadProgress reads this.resourceName at click time, i.e. the new name, so the abort 404s as well and the session is left behind for the retry loop to grind through.
Narrow, since Settings is a separate tab from the upload panel -- but model uploads default to a 2048 MiB per-file ceiling, so they run long enough for a tab switch to be realistic.
Cheapest fix is to disable the name field while activeCount > 0. Otherwise the panel needs to abort in-flight uploads under the old name before the rename lands.
| this.singleFileUploadMaxSizeMiB = this.endpoint.defaultMaxFileSizeMiB; | ||
| this.adminSettingsService | ||
| .getPublicSetting("dataset_single_file_upload_max_size_mib") | ||
| .getPublicSetting(this.endpoint.maxFileSizeSettingKey) |
There was a problem hiding this comment.
The PR description says the extracted upload code read single_file_upload_max_size_mib, "a key that does not exist", and that every dataset upload would have silently fallen back to 20 MiB.
I can't find that in this diff. The line this replaces already read dataset_single_file_upload_max_size_mib, and main uses the prefixed keys in files-uploader, dataset-detail and admin-settings alike -- nothing in the 32 files changes an unprefixed key. I assume it was true against an earlier commit on the branch.
Worth correcting in the description, otherwise reviewers go hunting for a fix that isn't in the diff. The endpoint constants themselves are right: all eight keys and both defaults match UploadLimits.scala.
The upload engine captures the resource name when an upload starts, so renaming mid-flight leaves the remaining part and finish calls addressed to a model that no longer answers to that name. Aborting cannot rescue it either: the abort reads the name at click time, i.e. the new one, so it 404s too and the multipart session is left for the retry loop to grind through. The panel now reports whether anything is in flight and the Settings tab disables the name field until it is idle, with onSaveModelName guarding the same invariant. The dataset page has the same bug and is left alone here, queued with the other pre-existing dataset fixes. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Head branch was pushed to by a user without write access
3 participants
What changes were proposed in this PR?
Models can now be uploaded to and versioned from the UI. PR 3 of the ML-model frontend series, on top of #8068 and #8072, both merged.
What you can do: upload files to a model, watch them upload, revert or delete staged changes, and commit them as a version. A new Settings tab edits the model's name, description, framework and format. Admin → Settings gains a Model card for the model upload limits.
1. Refactor — no behavior change. The dataset detail page owned the whole upload machinery. This pulls it out into resource-agnostic pieces, addressed by a
FileResourceEndpointdescriptor (base path, name query param, and the foursite_settingskeys):service/user/file-resource/multipart-upload.service.ts— the ~280-line multipart engine, out ofDatasetServiceservice/user/file-resource/staged-file.service.ts— diff, revert, deletecomponent/user/version-uploader/— the whole "Create New Version" panel (file picker, progress panels, staged list, version box)component/user/staged-objects-list/— moved out ofuser-dataset/2. Feature — models.
MODEL_FILE_RESOURCE_ENDPOINT,ModelServiceversion/framework/format calls, the model page's upload panel and Settings tab, and the Admin Model upload card.Two bugs fixed along the way.
Deliberately not here: Access & visibility (public/private, downloadable) lands in PR 4 — both can be set when creating a model, just not changed afterwards yet. Delete stays on the Models list card. View and like counts stay static
0until models reach the hub in PR 5.Any related issues, documentation, discussions?
Closes #6499.
Follows #8068 (Models page) and #8072 (model detail page), both merged. Does not touch unified search, so it does not depend on #7930.
How was this PR tested?
532 tests pass across the thirteen affected specs, including 72 in the new
version-uploader.component.spec.ts.Every dataset upload test still exists — they moved to the component that now owns that code rather than being deleted. That includes the three
loadUploadSettingsfallback tests, which pin the invariant the code comments warn about: a failed or unparsable settings fetch must leave the tuning fields alone, because a NaN concurrency limit stalls the queue outright (activeUploads < NaNis never true). TwongOnInitassertions changed: the dataset page no longer fetches upload tuning itself, so it no longer callsgetPublicSettingon load; the panel does, when it renders. No dataset behavior changed.New coverage: the model endpoint and its own settings keys, version creation, framework/format editing with rollback on failure, and the Admin upload cards run against both resource families so one cannot write the other's keys.
Was this PR authored or co-authored using generative AI tooling?
Generated-by: Claude Code (Claude Opus 5)