Fury0508 commented on PR #60728:
URL: https://github.com/apache/airflow/pull/60728#issuecomment-3809099804

   > > alwas need to re-sync my UV venv... is it intended by pre-commit checks 
to change / alter venvs? Or shall a temporary secondary be created for this 
check then?
   > 
   > Yeah, I also feel a bit strange at the first glance, which might also 
introduces some dependencies environment race condition issue from my 
perspective (e.g. other in-container pre-commit checks depend on the dev 
dependencies while current script is running).
   > 
   > I think the better way might be
   > 
   > 1. Creating a new `venv` under a tmp dir and install the dependencies 
without dev
   > 2. Extract the current `__main__` logic as another function (excluding the 
`sync_dependencies_without_dev` call) into a separate function named 
`provider_yaml_files_check`.
   > 3. Run `sync_dependencies_without_dev` for the `venv` under tmp dir.
   > 4. Run `provider_yaml_files_check` as a subprocess using the virtual 
environment we just created under the temporary path, ensuring that it runs 
without development dependencies (for example, by overriding ‎`PYTHONPATH`, 
etc.).
   > 
   > to avoid the race condition and environment side effect.
   
   Thanks for the detailed feedback @jason810496! You're absolutely right about 
the race condition and environment side effects.
   
   I'll refactor to use an isolated temporary venv as you suggested:
   1. Create temp venv with non-dev dependencies
   2. Extract validation logic to `provider_yaml_files_check()` 
   3. Run as subprocess using the temp environment
   
   Does this sound like the right direction? Happy to implement if this 
approach works for the team.


-- 
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: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]

Reply via email to