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.

Sign up for GitHub

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

Jump to bottom

Rename Stack to Stacks and Fix Bugs #107

Merged
merged 8 commits into from
Oct 19, 2023
Merged

Rename Stack to Stacks and Fix Bugs #107

merged 8 commits into from
Oct 19, 2023

Conversation

arpitjasa-db
Copy link
Collaborator

This PR renames instances of MLOps Stack to MLOps Stacks for consistency. It also fixes various bugs, and bumps the cluster source version to 0.2.

mingyu89
mingyu89 approved these changes Oct 18, 2023
View reviewed changes
Copy link
Contributor

@mingyu89 mingyu89 left a comment

Choose a reason for hiding this comment

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

Looks good to me. Adding more reviewers to give us more confidence.

...ame_alphanumeric_underscore` .}}/deployment/batch_inference/notebooks/BatchInference.py.tmpl
@@ -34,7 +34,7 @@ dbutils.widgets.text(
{{else}}
# Unity Catalog registered model name to use for the trained mode.
dbutils.widgets.text(
"model_name", "dev.{{template `schema_name` .}}.{{template `model_name` .}}", label="Model Name"
"model_name", "dev.{{template `schema_name` .}}.{{template `model_name` .}}", label="Full (Three-Level) Model Name"
Copy link
Contributor

Choose a reason for hiding this comment

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

The variable description looks good. I'm more referring to the model_name variable name. We can also wait and see if there's customer feedback about the name.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Yeah but then we'd have to add an if/else condition around every time that model_name is used on whether or not UC Models is enabled, and I didn't want to risk missing one of those this close to public preview.

README.md
you can e.g. copy just the GitHub Actions workflows under `.github` or ML resource configs
under ``{{.input_root_dir}}/{{template `project_name_alphanumeric_underscore` .}}/resources``
and ``{{.input_root_dir}}/{{template `project_name_alphanumeric_underscore` .}}/bundle.yml`` into your existing project.
and ``{{.input_root_dir}}/{{template `project_name_alphanumeric_underscore` .}}/databricks.yml`` into your existing project.
Copy link
Contributor

Choose a reason for hiding this comment

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

👍 Thanks

README.md
@@ -47,7 +47,7 @@ Please follow [the instruction](https://docs.databricks.com/en/dev-tools/cli/dat

To create a new project, run:

databricks bundle init https://github.com/databricks/mlops-stack
databricks bundle init mlops-stacks
Copy link
Contributor

Choose a reason for hiding this comment

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

This is awesome. Shall we cut a release and update the alias pointing to the release version later?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Yup! We'll be cutting the MLOps Stacks v0.2 release after this PR merges, and deco team has plans to add this semantic versioning to the bundle aliases.

stack-customization.md
* Logic to trigger model deployment through REST API calls to your CD system, when model training completes.
This logic is currently captured in ``template/{{.input_root_dir}}/{{template `project_name_alphanumeric_underscore` .}}/deployment/model_deployment/notebooks/TriggerModelDeploy.py``
This logic is currently captured in ``template/{{.input_root_dir}}/{{template `project_name_alphanumeric_underscore` .}}/deployment/model_deployment/notebooks/ModelDeployment.py``
Copy link
Contributor

Choose a reason for hiding this comment

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

This issue must be from long time ago 🤦 .

...ut_root_dir}}/{{template `project_name_alphanumeric_underscore` .}}/resources/README.md.tmpl
@@ -69,7 +69,7 @@ Alternatively, you can use the other approaches described in the [databricks CLI
### Validate and provision ML resource configurations
1. After installing the databricks CLI and creating the `DATABRICKS_TOKEN` env variable, change to the `{{template `project_name_alphanumeric_underscore` .}}` directory.
2. Run `databricks bundle validate` to validate the Databricks resource configurations.
3. Run `databricks bundle deploy` to provision the Databricks resource configurations to the dev workspace. The resource configurations and your ML code will be copied together to the dev workspace. The defined resources such as Databricks Workflows, MLflow Model and MLflow Experiment will be provisioned according to the config files under `{{template `project_name_alphanumeric_underscore` .}}/databricks-resource`.
3. Run `databricks bundle deploy` to provision the Databricks resource configurations to the dev workspace. The resource configurations and your ML code will be copied together to the dev workspace. The defined resources such as Databricks Workflows, MLflow Model and MLflow Experiment will be provisioned according to the config files under `{{template `project_name_alphanumeric_underscore` .}}/resources`.
Copy link
Contributor

Choose a reason for hiding this comment

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

Thanks

doc-images/mlops-stacks.png Outdated
Copy link
Contributor

Choose a reason for hiding this comment

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

It's 9x larger. I don't think it's a big deal.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Yeah I had to take the original image, overlay the new text, and then take a screenshot since I couldn't find the original file where we created the diagram 🤦‍♂️

...emplate `project_name_alphanumeric_underscore` .}}/deployment/batch_inference/README.md.tmpl
@@ -47,7 +47,7 @@ df = spark.table(
).drop("fare_amount")

df.write.mode("overwrite").saveAsTable(
{{ if (eq .input_include_models_in_unity_catalog `yes`) }}name="hive_metastore.default.taxi_scoring_sample"
{{ if (eq .input_include_models_in_unity_catalog `no`) }}name="hive_metastore.default.taxi_scoring_sample"
Copy link
Contributor

Choose a reason for hiding this comment

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

👍

shaotong-db
shaotong-db approved these changes Oct 18, 2023
View reviewed changes
Copy link

@shaotong-db shaotong-db left a comment

Choose a reason for hiding this comment

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

LGTM. Thank you Arpit!

zhe-db
zhe-db approved these changes Oct 19, 2023
View reviewed changes
Copy link
Contributor

@zhe-db zhe-db left a comment

Choose a reason for hiding this comment

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

🚀🚀🚀

@arpitjasa-db arpitjasa-db merged commit 5d940d5 into main Oct 19, 2023
1 check passed
@arpitjasa-db arpitjasa-db deleted the bugFixAndRename branch October 19, 2023 19:37
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@shaotong-db shaotong-db shaotong-db approved these changes

@vladimirk-db vladimirk-db vladimirk-db approved these changes

@lennartkats-db lennartkats-db lennartkats-db approved these changes

@zhe-db zhe-db zhe-db approved these changes

@mingyu89 mingyu89 mingyu89 approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
6 participants
@arpitjasa-db @mingyu89 @zhe-db @lennartkats-db @vladimirk-db @shaotong-db