-
-
Notifications
You must be signed in to change notification settings - Fork 2k
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: Ensure Python version matches version used to serialize credential provider #19375
Conversation
ser_py_version | ||
"current Python version {:?} does not match the Python version used to serialize the UDF {:?}", | ||
(3, cur_py_version[0], cur_py_version[1]), | ||
(3, ser_py_version[0], ser_py_version[1] ) |
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.
Changed this to also check the micro version - from research pickle requires the exact same Python version.
was added from #19175
@@ -141,8 +141,8 @@ impl ColumnsUdf for PythonUdfExpression { | |||
.getattr("dumps") | |||
.unwrap(); | |||
let pickle_result = pickle.call1((self.python_function.clone_ref(py),)); | |||
let (dumped, use_cloudpickle, py_version) = match pickle_result { |
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.
Still serialize the correct Python version even if we are using cloudpickle
@@ -57,14 +57,14 @@ impl PythonUdfExpression { | |||
// Handle pickle metadata | |||
let use_cloudpickle = buf[0]; | |||
if use_cloudpickle != 0 { |
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.
I'm not sure about this, but the existing code for PythonUDF deserialization doesn't enforce the Python version equality if it was serialized with cloudpickle. Maybe we should?
e284b51
to
3f22305
Compare
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #19375 +/- ##
==========================================
+ Coverage 80.18% 80.21% +0.02%
==========================================
Files 1523 1523
Lines 209897 209999 +102
Branches 2434 2434
==========================================
+ Hits 168314 168441 +127
+ Misses 41028 41002 -26
- Partials 555 556 +1 ☔ View full report in Codecov by Sentry. |
No description provided.