Skip to content

Conversation

@shreyas-goenka
Copy link
Contributor

@shreyas-goenka shreyas-goenka commented Sep 29, 2025

Changes

This PR adds support for registered models, matching the same behaviour as TF. This PR also adds test server handles for registered models and UC catalogs, as was necessary for the integration test.

Why

To complete coverage for direct deployment.

Tests

New integration test.

@eng-dev-ecosystem-bot
Copy link
Collaborator

eng-dev-ecosystem-bot commented Sep 29, 2025

Run: 18162343450

Env ✅​pass 🔄​flaky 🙈​skip
✅​ aws linux 320 537
✅​ aws windows 321 536
✅​ aws-ucws linux 436 433
✅​ aws-ucws windows 437 432
✅​ azure linux 320 536
✅​ azure windows 321 535
✅​ azure-ucws linux 436 432
🔄​ azure-ucws windows 435 2 431
✅​ gcp linux 319 538
✅​ gcp windows 320 537
Test Name azure-ucws windows
TestFilerWorkspaceNotebook 🔄​flaky
TestFilerWorkspaceNotebook/rJupyterNb.ipynb 🔄​flaky

@shreyas-goenka shreyas-goenka changed the title [wip] add support for registered_models for direct deployment Add support for registered models for direct deployment Sep 29, 2025
Deleting files...
Destroy complete!

>>> [CLI] schemas delete mycatalog-[UNIQUE_NAME].myschema-[UNIQUE_NAME] --force
Copy link
Contributor

Choose a reason for hiding this comment

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

As discussed offline, would be great to use acceptance/bundle/resources/<resource_name>/ schema.

bundle/deploy is really focussed on deploy command but we're also interested in plan/json plan/destroy/summary.

another aspect is to use smaller tests which is good for perf and debugging:

See how this test is organized:

 ~/work/cli/acceptance/bundle/resources % tree -d volumes
volumes
├── change-comment
├── change-name
├── change-schema-name
├── remote-change-name
├── remote-delete
└── set-storage-location

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I'll address this in a followup PR.

@@ -0,0 +1,2 @@
Cloud = true
Copy link
Contributor

Choose a reason for hiding this comment

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

Appreciate making it both local and integration test, that adds to confidence.

trap cleanup EXIT

deploy_registered_model() {
trace $CLI bundle plan
Copy link
Contributor

Choose a reason for hiding this comment

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

recording "bundle debug plan" would be nice as well.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I'll address this in a followup PR.


deploy_registered_model() {
trace $CLI bundle plan
trace $CLI bundle deploy
Copy link
Contributor

Choose a reason for hiding this comment

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

recording non-GET requests would be useful to ensure terraform and direct implementations match. If there is too much difference in payload, it can either be cleaned up or we can just record paths without the payload.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I'll address this in a followup PR.

Copy link
Contributor

@denik denik left a comment

Choose a reason for hiding this comment

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

The implementation looks solid, there are comments on test organization, can be done here or as a follow up.

@shreyas-goenka shreyas-goenka added this pull request to the merge queue Oct 1, 2025
@github-merge-queue github-merge-queue bot removed this pull request from the merge queue due to a conflict with the base branch Oct 1, 2025
@shreyas-goenka shreyas-goenka added this pull request to the merge queue Oct 1, 2025
Merged via the queue into main with commit 877d53b Oct 1, 2025
13 checks passed
@shreyas-goenka shreyas-goenka deleted the direct/registered-model branch October 1, 2025 15:54
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.

4 participants