uranusjr commented on pull request #16954:
URL: https://github.com/apache/airflow/pull/16954#issuecomment-878984200


   The argument names look very wrong to me. `--trusted-host` is for overriding 
the HTTPS validation logic, and this is reflected neither by the name 
`repository_url` and its description in the docstring. If we are going to 
expose this, the argument name should be transparently `trusted_host` (with 
appropriate documentation).
   
   Similarly, the name `index_url` seems to indicate the URL will be used to 
download all packages. This is not the case, since the value is currently 
passed to `--extra-index-url`, which will be used *in additional to PyPI*. 
Again, the argument name should be transparently mapped IMO.
   
   Furthermore, since [using `--extra-index-url` to distribute private packages 
is bad and introduces security 
vulnerabilities](https://github.com/pypa/pip/issues/8606), everyone except a 
very limited number of people should use `--index-url` to override the entire 
index instead. So Airflow should either only expose `--index-url`, or implement 
a wrapper to convert what the user wants into something can be configured via 
`--index-url`.
   
   Lastly, this (and the index URL value) will result in a syntax error if the 
URL contains `=` (which is valid). It is better to form the options like this: 
`f"--option={url}"`.


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