-
Notifications
You must be signed in to change notification settings - Fork 4k
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
Remove duplicate _SPARK_MODEL_PATH_SUB
#6683
Conversation
Signed-off-by: zero <164434719@qq.com>
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.
thanks @gwy1995 !
@bbarnes52 @gwy1995 Looks like a number of Spark tests fail with this change. Can we confirm whether this is a test-only issue (i.e. the tests' expectations are incorrect) or a problematic change in behavior? |
The tests' expectations are incorrect due to duplicate _SPARK_MODEL_PATH_SUB mlflow#6683 Signed-off-by: zero <164434719@qq.com>
@dbczumar I think the tests' expectations are incorrect. I have a new commit on them. |
Thanks @gwy1995 @bbarnes52 Can you take a look and also verify that this duplicate path was introduced in MLflow 1.28.0 and wasn't present before? @BenWilson2 flagged that we should be particularly careful about backwards compatibility here - if 1.28.0 introduced a regression, we should absolutely fix it. If this duplicate path component has been around for a long time, we should maintain backwards compatibility. |
Confirmed offline with @bbarnes52 that this is indeed a regression in MLflow 1.28.0. Merging! cc @BenWilson2 |
* mlflow#6682 Remove duplicate `_SPARK_MODEL_PATH_SUB` Signed-off-by: zero <164434719@qq.com> * update tests expectations The tests' expectations are incorrect due to duplicate _SPARK_MODEL_PATH_SUB mlflow#6683 Signed-off-by: zero <164434719@qq.com> Signed-off-by: zero <164434719@qq.com>
Signed-off-by: zero 164434719@qq.com
Related Issues/PRs
Fix #6682
What changes are proposed in this pull request?
Remove duplicate
_SPARK_MODEL_PATH_SUB
inmlflow.spark.log_model
Does this PR change the documentation?