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

Add integration test for mlops-stacks initialization #1155

Merged
merged 11 commits into from
Mar 12, 2024
Merged

Conversation

shreyas-goenka
Copy link
Contributor

@shreyas-goenka shreyas-goenka commented Jan 25, 2024

Changes

This PR:

  1. Adds an integration test for mlops-stacks that checks the initialization and deployment of the project was successful.
  2. Fixes a bug in the initialization of templates from non-tty. We need to process the input parameters in order since their descriptions can refer to input parameters that came before in the interactive UX.

Tests

The integration test passes in CI.

internal/init_test.go Outdated Show resolved Hide resolved
internal/init_test.go Show resolved Hide resolved
internal/init_test.go Outdated Show resolved Hide resolved
internal/init_test.go Show resolved Hide resolved
initConfig := map[string]string{
"input_project_name": "project_name",
"input_root_dir": "repo_name",
"input_include_models_in_unity_catalog": "no",
Copy link
Contributor

Choose a reason for hiding this comment

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

Any chance we could run UC models as well? If not a separate test case, then prefer making this yes since it's our more important journey

Copy link
Contributor Author

Choose a reason for hiding this comment

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

UC tests are possible, but the test will require and the equivalent assertions will need make finer grained assumptions. Needing to set up schema, setting more input parameters like the schema name and catalog name and specifying a target deployment explicitly are a couple I noticed.

Given this level of dependencies on the details, I'd prefer to have the UC test in the MLOps repo itself, so that the tests are managed alongside the template changes.

Copy link
Contributor

Choose a reason for hiding this comment

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

Kk makes sense, can we keep a UC initialization test then? We don't have to deploy/run any of the workflows, but good to make sure we can at least initialize the template

@codecov-commenter
Copy link

codecov-commenter commented Mar 11, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 52.31%. Comparing base (945d522) to head (49bf903).

Additional details and impacted files
@@           Coverage Diff           @@
##             main    #1155   +/-   ##
=======================================
  Coverage   52.30%   52.31%           
=======================================
  Files         316      316           
  Lines       17980    17983    +3     
=======================================
+ Hits         9405     9408    +3     
  Misses       7882     7882           
  Partials      693      693           

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

@shreyas-goenka shreyas-goenka added this pull request to the merge queue Mar 12, 2024
Merged via the queue into main with commit d4329f4 Mar 12, 2024
4 checks passed
@shreyas-goenka shreyas-goenka deleted the mlops-test branch March 12, 2024 14:26
@pietern pietern mentioned this pull request Mar 25, 2024
pietern added a commit that referenced this pull request Mar 25, 2024
CLI:
 * Propagate correct `User-Agent` for CLI during OAuth flow ([#1264](#1264)).
 * Add usage string when command fails with incorrect arguments ([#1276](#1276)).

Bundles:
 * Include `dyn.Path` as argument to the visit callback function ([#1260](#1260)).
 * Inline logic to set a value in `dyn.SetByPath` ([#1261](#1261)).
 * Add assertions for the `dyn.Path` argument to the visit callback ([#1265](#1265)).
 * Add `dyn.MapByPattern` to map a function to values with matching paths ([#1266](#1266)).
 * Filter current user from resource permissions ([#1262](#1262)).
 * Retain location annotation when expanding globs for pipeline libraries ([#1274](#1274)).
 * Added deployment state for bundles ([#1267](#1267)).
 * Do CheckRunningResource only after terraform.Write ([#1292](#1292)).
 * Rewrite relative paths using `dyn.Location` of the underlying value ([#1273](#1273)).
 * Push deployment state right after files upload ([#1293](#1293)).
 * Make `Append` function to `dyn.Path` return independent slice ([#1295](#1295)).
 * Move bundle tests into bundle/tests ([#1299](#1299)).
 * Upgrade Terraform provider to 1.38.0 ([#1308](#1308)).

Internal:
 * Add integration test for mlops-stacks initialization ([#1155](#1155)).
 * Update actions/setup-python to v5 ([#1290](#1290)).
 * Update codecov/codecov-action to v4 ([#1291](#1291)).

API Changes:
 * Changed `databricks catalogs list` command.
 * Changed `databricks online-tables create` command.
 * Changed `databricks lakeview publish` command.
 * Added `databricks lakeview create` command.
 * Added `databricks lakeview get` command.
 * Added `databricks lakeview get-published` command.
 * Added `databricks lakeview trash` command.
 * Added `databricks lakeview update` command.
 * Moved settings related commands to `databricks settings` and `databricks account settings`.

OpenAPI commit 93763b0d7ae908520c229c786fff28b8fd623261 (2024-03-20)

Dependency updates:
 * Bump golang.org/x/oauth2 from 0.17.0 to 0.18.0 ([#1270](#1270)).
 * Bump golang.org/x/mod from 0.15.0 to 0.16.0 ([#1271](#1271)).
 * Update Go SDK to v0.35.0 ([#1300](#1300)).
 * Update Go SDK to v0.36.0 ([#1304](#1304)).
github-merge-queue bot pushed a commit that referenced this pull request Mar 25, 2024
CLI:
* Propagate correct `User-Agent` for CLI during OAuth flow
([#1264](#1264)).
* Add usage string when command fails with incorrect arguments
([#1276](#1276)).

Bundles:
* Include `dyn.Path` as argument to the visit callback function
([#1260](#1260)).
* Inline logic to set a value in `dyn.SetByPath`
([#1261](#1261)).
* Add assertions for the `dyn.Path` argument to the visit callback
([#1265](#1265)).
* Add `dyn.MapByPattern` to map a function to values with matching paths
([#1266](#1266)).
* Filter current user from resource permissions
([#1262](#1262)).
* Retain location annotation when expanding globs for pipeline libraries
([#1274](#1274)).
* Added deployment state for bundles
([#1267](#1267)).
* Do CheckRunningResource only after terraform.Write
([#1292](#1292)).
* Rewrite relative paths using `dyn.Location` of the underlying value
([#1273](#1273)).
* Push deployment state right after files upload
([#1293](#1293)).
* Make `Append` function to `dyn.Path` return independent slice
([#1295](#1295)).
* Move bundle tests into bundle/tests
([#1299](#1299)).
* Upgrade Terraform provider to 1.38.0
([#1308](#1308)).

Internal:
* Add integration test for mlops-stacks initialization
([#1155](#1155)).
* Update actions/setup-python to v5
([#1290](#1290)).
* Update codecov/codecov-action to v4
([#1291](#1291)).

API Changes:
 * Changed `databricks catalogs list` command.
 * Changed `databricks online-tables create` command.
 * Changed `databricks lakeview publish` command.
 * Added `databricks lakeview create` command.
 * Added `databricks lakeview get` command.
 * Added `databricks lakeview get-published` command.
 * Added `databricks lakeview trash` command.
 * Added `databricks lakeview update` command.
* Moved settings related commands to `databricks settings` and
`databricks account settings`.

OpenAPI commit 93763b0d7ae908520c229c786fff28b8fd623261 (2024-03-20)

Dependency updates:
* Bump golang.org/x/oauth2 from 0.17.0 to 0.18.0
([#1270](#1270)).
* Bump golang.org/x/mod from 0.15.0 to 0.16.0
([#1271](#1271)).
* Update Go SDK to v0.35.0
([#1300](#1300)).
* Update Go SDK to v0.36.0
([#1304](#1304)).
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.

5 participants