[mmm-18320] operator create registered model registered model version

[paulsliusar]

This repository is public. Do not put here any private DataRobot or customer's data: code, datasets, model artifacts, .etc.

Summary

This operator is created to cover functionality of creating a registered model version for several different model types

• Leaderboard Model
• Custom Model
• External Model
  for detailed description you can follow update in readme file and for more details on client side

Rationale

[engprod-2]

The Needs Review labels were added based on the following file changes.

Team @datarobot/machine-learning-development (#machine-learning-dev) was assigned because of changes in files:

README.md
datarobot_provider/operators/model_registry.py
tests/unit/operators/test_model_registry.py

If you think that there are some issues with ownership, please discuss with C&A domain at #sdtk slack channel and create PR to update DRCODEOWNERS\CODEOWNERS file.

[Zotkin]

Please take a look at the comments regarding registered model name/id (The confusion is probably coming from the bug in the documentation example code that needs to be fixed).
Additional question: all model package creation endpoints accept more parameters than implemented here (for example, external models allow for training data assignment when creation model package). Is this something you are planning to address in the future?

[Zotkin]

The logic and text description regarding the registerd_model_name and registered_model_id are incorrect. Those parameters are mutually exclusive and do not depend on underlying model type.
When providing registered model name you are saying: please take this model, create a model package (registered model version) and assign it to new registered model with given name.
When providing registered_model_id you are assigning newly created model package to the existing registered model.

[paulsliusar]

you are right will correct this, as for other parameters that are optional will extend methods to pass them as kwargs, and update readme file

[Zotkin]

This parameter has nothing to do with leaderboard or not.

[Zotkin]

Given the above comments, this should probably be reworked with regards to registered_model_name/id

[tsdaemon]

Please add a note to CHANGES.
Also, I believe there is no need to write about every operator in README. @mjnitz02 prepared autogenerated documentation, and we can release it with the first new versionn.

[c-h-russell-walker]

You should be able to remove this boiler plate code thanks to using the BaseDatarobotOperator 😎

Suggested change

	template_fields_renderers: dict[str, str] = {}
	template_ext: Sequence[str] = ()
	ui_color = "#f4a460"

[c-h-russell-walker]

Same here with some boiler plate code - it could be useful to check against this PR's changes for examples:
https://github.com/datarobot/airflow-provider-datarobot/pull/135/files

Suggested change

	datarobot_conn_id: str = "datarobot_default",
	**kwargs: Any,
	) -> None:
	super().__init__(**kwargs)
	self.model_version_params = model_version_params
	self.datarobot_conn_id = datarobot_conn_id
	**kwargs: Any,
	) -> None:
	super().__init__(**kwargs)
	self.model_version_params = model_version_params

[c-h-russell-walker]

Also taken care of in the newer base class 👍

Suggested change

DataRobotHook(datarobot_conn_id=self.datarobot_conn_id).run()

[andrius-senulis]

LGTM

[andrius-senulis]

This happens in the base operator

[c-h-russell-walker]

Could you move these lines into the validate() method now provided by the newer base class?

I think lines should work as is as long as you alter:
if not model_type:
to be:
if not self.model_version_params.get("model_type"):

[c-h-russell-walker]

Looks like this new file is missing a copyright - I had assumed each new file should have that.

CC @mjnitz02 maybe we should be sure (assuming we need copyrights) that we're running a copyright checker as part of PR checks?

[c-h-russell-walker]

Sorry, probably last comment related to the new base class, but I think we can also remove this.

Suggested change

:param datarobot_conn_id: Airflow connection ID for DataRobot.

[Zotkin]

Looks good in terms of implementation. Given that this PR includes a lot of customer facing strings, I'd suggest getting a review from @n-aylward .

[n-aylward]

@Zotkin @paulsliusar Apologies. I was out this morning for a doctor's appointment. Since this is already merged, perhaps we should just have someone from the doc team (potentially me) review the new content in the course of the documentation work for DOC-7718.