-
Notifications
You must be signed in to change notification settings - Fork 170
[tools] Introduce opm (one package manger) #16336
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: master
Are you sure you want to change the base?
Conversation
|
Please use |
tools/opm/import.py
Outdated
| print(f"Downloading model files for {model_id} into {target_dir}...") | ||
|
|
||
| # Patch httpx to disable SSL verification and forward proxy if set | ||
| import httpx |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why don't you put 'import' to top of the script?
tools/opm/import.py
Outdated
| # Patch httpx to disable SSL verification and forward proxy if set | ||
| import httpx | ||
| original_client = httpx.Client | ||
| proxy = os.getenv("HTTPS_PROXY") or os.getenv("HTTP_PROXY") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Proxy settings can also work with lower case too, (e.g. 'http_proxy').
I explored a more generic approach and found one that utilizes 'urllib.request'.
import urllib.request
proxy = urllib.request.getproxies()It works in a case-insensitive way.
| # PR-related constants (used only in init.py) | ||
| PR_WORKTREE = "_pr_16233" | ||
| PR_BRANCH = "pr-16233" | ||
| PR_REF = "refs/pull/16233/head" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
In PEP 8, it said
"Imports are always put at the top of the file, just after any module comments and docstrings, and before module globals and constants."
So, it's better to put them after following imports.
@hseok-oh Thank you for review! 👍 It would be good to use It would be best to fix the requirements of Anyway, if we don't care about spending time and money, I would update (ADD) I introduced |
|
@batcheu Thank you for review! I never thought it is not recommended way in Python. I updated as you commented. I am not sure PEP-8 is happy about the code. |
It introduces opm tools for building package from pytorch. ONE-DCO-1.0-Signed-off-by: Sanggyu Lee <sg5.lee@samsung.com>
|
I added
|
It introduces opm tools for building package from pytorch.
ONE-DCO-1.0-Signed-off-by: Sanggyu Lee sg5.lee@samsung.com