[functions] Use std lib zipfile to check if python function is a .py or a .zip#9773
[functions] Use std lib zipfile to check if python function is a .py or a .zip#9773mauza wants to merge 2 commits intoapache:masterfrom mauza:master
Conversation
When running a python function in pulsar on a broker don't rely on the extension to tell if the file is a zipfile or not but use the built in zipfile.is_zipfile function to check.
|
After testing with this change locally it doesn't seem to be helping my problem. Closing PR. |
|
I actually think this still may be a fix. |
| zpfile.extractall(os.path.dirname(str(args.py))) | ||
| sys.path.insert(0, os.path.dirname(str(args.py))) | ||
| elif os.path.splitext(str(args.py))[1] == '.zip': | ||
| elif zipfile.is_zipfile(str(args.py)): |
There was a problem hiding this comment.
Yeah, I'm new to contributing so any direction on how to go about doing that would be helpful, but I'll dive in see if I can figure out how to create some tests around this.
There was a problem hiding this comment.
You can take a look at test_python_instance.py.
There was a problem hiding this comment.
@zymap I think we don't test 3rd-party dependency for their own functionality :)
|
The pr had no activity for 30 days, mark with Stale label. |
|
It seems the author deleted the patch branch. Closed as stale. |
|
Although, I think it's good to have. So if anyone continue this patch, I'm glad to review. |
5 participants
When running a python function in pulsar on a broker don't rely on the extension to tell if the file is a zipfile or not but use the built in zipfile.is_zipfile(filepath) function to check.
Motivation
Automation using the pulsar admin api saves python zip files as .py files which breaks when running the function on a broker because python_instance_main.py only checks if they file has a .zip extension for it to extract all the contents.
Modifications
Changed the conditional that checks for a .zip extensions to now use zipfile.is_zipfile
Verifying this change
(Please pick either of the following options)
This change is a trivial rework / code cleanup without any test coverage.
(or)
This change is already covered by existing tests, such as (please describe tests). <---- I'm guessing
(or)
This change added tests and can be verified as follows:
(example:)
Does this pull request potentially affect one of the following parts:
If
yeswas chosen, please highlight the changesDocumentation