-
Notifications
You must be signed in to change notification settings - Fork 1
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
Fix uniqueness constraint with .auth.db.models.SqlUser.username
#52
Conversation
5f78a75
to
9da5b07
Compare
mlflow_cratedb/patch/mlflow/model.py
Outdated
TODO: There are two more unique constraints defined on the MLflow data model. | ||
- SqlExperimentPermission: "experiment_id", "user_id" | ||
- SqlRegisteredModelPermission: "name", "user_id" | ||
""" | ||
from mlflow.store.model_registry.dbmodels.models import SqlRegisteredModel | ||
from mlflow.store.tracking.dbmodels.models import SqlExperiment | ||
from mlflow.server.auth.db.models import SqlUser | ||
|
||
listen(SqlExperiment, "before_insert", check_uniqueness_factory(SqlExperiment, "name")) | ||
listen(SqlRegisteredModel, "before_insert", check_uniqueness_factory(SqlRegisteredModel, "name")) | ||
listen(SqlUser, "before_insert", check_uniqueness_factory(SqlUser, "username")) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Other than the constraint on SqlUser
added on behalf of this patch, there are two more to be addressed. Both can be found within *Permission
types of classes. I don't know if you are using this subsystem of MLflow, @andnig?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The emulative implementation currently is not able to handle composite unique key constraints, so I've created a corresponding issue to track it. Even more yak shaving ahead.
8b4fb77
to
bc06315
Compare
bc06315
to
1e39d65
Compare
About
Just a minor followup to GH-46.
Trivia
The first CI run on this patch experienced an interesting fluke with CrateDB. We added a report at GH-53.