-
Notifications
You must be signed in to change notification settings - Fork 14.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
Access function to be pickled as attribute, not method, to avoid error. #8823
Conversation
self._processor_factory, | ||
# getattr allows pickling by making _processor_factory get | ||
# treated as a function, not an instance method. | ||
getattr(self, "_processor_factory"), |
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.
LOLwhat. Thanks pickle.
Is _processor_factory marked as @staticmethod? If not it should be, and then probably this becomes
getattr(self, "_processor_factory"), | |
type(self)._processor_factory, |
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.
@ashb I agree, for purposes of consistency. But I actually think getattr
is more clear. IMO type()
makes it look like it's a class attribute, which isn't the case.
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.
Actually, that's not true. Apparently pylint expects type(self)
to access an explicitly declared class attribute, whereas getattr
has no such requirement. Since _processor_factory
is declared at runtime, and is really a dynamic instance attribute, I think it makes sense to stay with getattr
.
Also, I tried declaring _processor_factory
at the class level and using type(self)._processor_factory
, but it lead to an error in the child process. This didn't seem worth debugging just to use type(self)
.
…r. (apache#8823) * Access function to be pickled as attribute, not method, to avoid error. * Access type attribute to allow pickling. * Use getattr instead of type(self) to fix linting error. (cherry picked from commit 7533378)
Currently, a static function is getting saved to
DagFileProcessorAgent
as an instance attribute, so it gets treated like an instance method when pickled, causing an error. By usinggetattr
to access the attribute we can avoid this issue.Related to issue #8674
Make sure to mark the boxes below before creating PR: [x]
In case of fundamental code change, Airflow Improvement Proposal (AIP) is needed.
In case of a new dependency, check compliance with the ASF 3rd Party License Policy.
In case of backwards incompatible changes please leave a note in UPDATING.md.
Read the Pull Request Guidelines for more information.