[GitHub] [airflow] o-nikolas commented on pull request #25780: Implement PythonOtherenvOperator

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

o-nikolas commented on PR #25780:
URL: https://github.com/apache/airflow/pull/25780#issuecomment-1229022042

    > I think I figured out a good proposal that fits very well both - requirements of being similar to virtualenv and being "correct" in terms of not referring to virtualenv.
   > 
   > My proposal (and it's already updated in the PR) is:
   > 
   >     * PythonOtherenvOperator
   > 
   >     * @task.otherenv decorator
   > 
   > 
   > I think this addresses all the concerns, it is short, easy to remember and use and also has very close resemblance to PythonVirtualenvOperator to show that it is closer to it than to PythonOperator and it does not imply Virtualenv.
   > 
   > Few doubts I had (and I made some choices that could be changed still):
   > 
   >     * PythonOtherEnvOperator vs PythonOtherenvOperator -> I think the latter is better even if slightly less "correct" - it also matches well the decorator (we have no casing in decorator by convention)
   > 
   >     * @task.python_otherenv vs. @task.otherenv  -> I think the latter is better: shorter and more close to @task.virtualenv too.
   > 
   >     * still there is one reference to virtualenv - we have `virtualenv_string_args` still created as 'global' variable accessible in the task. Changing it would be backwards-incompatible, and I think it's not worth to handle it differently.
   > 
   > 
   > Let me know what you think @o-nikolas @uranusjr @ashb. Does it look `good-enough` for all of you :)
   
   I think "Other" is a bit too vague. I personally prefer a name be a bit on the longer side and then be very declarative/clear, rather than saving a few characters and it becoming terse and vague. Maybe dropping the "Pre" from the last name would be a compromise: `PythonExistingVirtualenvOperator`? Even, though I personally like "Preexisting" (it's just three more letters long and it is  __very__ clear what the meaning is) I think "Existing" could be a good middle ground.


-- 
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.

To unsubscribe, e-mail: commits-unsubscribe@airflow.apache.org

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