potiuk commented on a change in pull request #20200:
URL: https://github.com/apache/airflow/pull/20200#discussion_r772547382



##########
File path: .github/workflows/build-images.yml
##########
@@ -297,7 +301,11 @@ jobs:
           rm -rf "scripts/ci"
           mv "main-airflow/scripts/ci" "scripts"
       - name: "Free space"
-        run: ./scripts/ci/tools/free_space.sh
+        run: ./scripts/ci/tools/freespace.py

Review comment:
       I think we should aim to install everywhere python with dependencies 
from 'dev/breeze". That was not needed  before, but if move all the "python" 
scripts we run in CI to "dev/breeze", then it will be easier to rely on 
whatever dependencies we have there (for example rich). 
   
   So I think we should really aim to have this everywhere:
   
   ```
    - uses: actions/setup-python@v2
       with:
         python-version: '3.7'
         cache: 'pip'
     - run: pip install ./dev/breeze
   ```
   
   Unfortunately, we will have to repeat it everywhere as there is no easy way 
currently to reuse parts of the jobs easily (as far as I know).
   
   Actually, you made me think a bit and I think we could do even better (that 
would be really nice!):
   
   1) you could move the "freespace.py" to "./dev/breeze/ci/freespace.py"
   
   2) we could add freespace.py as as "entrypoint"
   
   See here: 
https://github.com/apache/airflow/blob/4dd751d2fc3f091417146079d7d1b3ac6dd86c9c/dev/breeze/setup.cfg#L64
   
   We could do something like:
   ```
   console_scripts=
       Breeze2=airflow_breeze.breeze:main
       freespace=ci.freespace.main
   ```
   
   And that would be really nice, because after running:
   ```
   pip install ./dev/breeze
   ```
   
   the `freespace` script will become available on the PATH and you will be 
able to call it just like:
   
   ```
   run: freespace
   ```
   
   That woudl be really, really nice in the `pipx` scenario (@uranusjr !) which 
i start liking more and more, because in case of `pipx` installation, freespace 
will not ony become available in path but also will have autocomplete enabled 
after we add it to the autocomplete scripts. Maybe freespace is not best 
example but we have more "standalone" scripts that are not part of `breeze`  
that we could expose in similar way eventually.
   
   
   




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