[GitHub] [airflow] RikHeijdens edited a comment on pull request #12788: Fix plugin macros not being exposed through airflow.macros

[GitBox <gi...@apache.org>]

RikHeijdens edited a comment on pull request #12788:
URL: https://github.com/apache/airflow/pull/12788#issuecomment-738776508


   @mik-laj I noticed the change proposed here causes the `TestPythonVirtualenvOperator.test_airflow_context` test to fail. I believe this is happening because `airflow.macros` is being serialized as part of the context which is passed to the underlying subprocess. 
   
   The subprocess in which the Python callable runs however does not integrate the macros provided by plugins. I was able to get this test to pass locally by adding the following two lines of code to `airflow/utils/python_virtualenv_script.jinja2` (see https://github.com/RikHeijdens/airflow/commit/b2c035c87f9cfa95f6185b55c74a4b3d38d9fb63):
   
   ```python
   # Ensure airflow.macros has been loaded properly.
   import airflow.plugins_manager
   airflow.plugins_manager.integrate_macros_plugins()
   ```
   
   Doing this however, seems like a bit of a nasty solution to me and also causes additional test failures. I'm curious whether you have any thoughts on what would be a better solution? What is the use-case of allowing subprocess spawned by PythonVirtualenvOperator to access plugin-provided macros?
   
   Edit: I went ahead and removed `airflow.macros` from the set of `AIRFLOW_SERIALIZABLE_CONTEXT_KEYS` in PythonVirtualEnvironmentOperator. I'm not sure whether this can be considered as a backwards-incompatible change given that this functionality is currently broken, but hopefully this should allow all tests to pass at the very least.
   
   
   


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org