07.Version Cache - Support for PIP #14613
Conversation
|
@xumia can you please help to review? Thanks. |
| @@ -231,6 +250,7 @@ run_pip_command() | |||
| ([ "$para" == "-c" ] || [ "$para" == "--constraint" ]) && found=y | |||
| if [ "$para" == "install" ]; then | |||
| install=y | |||
| parameters[${count}]="install --trusted-host pypi.org --trusted-host files.pythonhosted.org --index-url https://pypi.org/simple " | |||
There was a problem hiding this comment.
Maybe we should not add the hard code reference to the pypi indexes, we want to make sure that the customized config work fine. Some of us have the security requirement to make sure using own pipy index, not pypi.org.
There was a problem hiding this comment.
Maybe we should not add the hard code reference to the pypi indexes, we want to make sure that the customized config work fine. Some of us have the security requirement to make sure using own pipy index, not pypi.org.
Can we make it configurable?
There was a problem hiding this comment.
removed the hardcoded reference
Kalimuthu-Velappan
force-pushed
the
VERSION_CACHE_PIP
branch
from
6602fc2 to
eba7cfc
Compare
|
|
||
| if [ ! -x "$REAL_COMMAND" ] && [ " $1" == "freeze" ]; then | ||
| return 1 | ||
| fi | ||
|
|
||
| if [ "$ENABLE_VERSION_CONTROL_PY" != "y" ]; then | ||
| if [[ "$SKIP_BUILD_HOOK" == Y || "$ENABLE_VERSION_CONTROL_PY" != "y" ]]; then |
| if [ "$ENABLE_VERSION_CONTROL_PY" != "y" ]; then | ||
| if [[ "$SKIP_BUILD_HOOK" == Y || "$ENABLE_VERSION_CONTROL_PY" != "y" ]]; then | ||
| if [ ! -z "$(get_version_cache_option)" ]; then | ||
| mkdir -p ${PIP_CACHE_PATH} |
There was a problem hiding this comment.
Why do we need to use mkdir multiple times: here (line 228) and at line 218 with sudo?
There was a problem hiding this comment.
Removed, done
| done | ||
|
|
||
| if [ "$found" == "n" ] && [ "$install" == "y" ]; then | ||
| parameters+=("-c") | ||
| parameters+=("${tmp_version_file}") | ||
| fi | ||
|
|
||
| $REAL_COMMAND "${parameters[@]}" | ||
| if [ ! -z "$(get_version_cache_option)" ]; then |
There was a problem hiding this comment.
syntax error: fi for this if is missed
| if [[ ! -e ${PIP_CACHE_PATH} ]]; then | ||
| ${SUDO} mkdir -p ${PIP_CACHE_PATH} | ||
| chmod 777 ${PIP_CACHE_PATH} | ||
| fi | ||
|
|
||
| if [ ! -x "$REAL_COMMAND" ] && [ " $1" == "freeze" ]; then |
There was a problem hiding this comment.
This code was not changed, but probably this condition is wrong dut to whitespace before $1.
| touch ${PIP_CACHE_PATH} | ||
| FUNLOCK ${PIP_CACHE_PATH} | ||
| else | ||
| $REAL_COMMAND ${parameters[@]} |
| $REAL_COMMAND "${parameters[@]}" | ||
| if [ ! -z "$(get_version_cache_option)" ]; then | ||
| FLOCK ${PIP_CACHE_PATH} | ||
| $REAL_COMMAND ${PKG_CACHE_OPTION} ${parameters[@]} |
There was a problem hiding this comment.
double quotes missed?
compare with line 200 e.g.
Kalimuthu-Velappan
force-pushed
the
VERSION_CACHE_PIP
branch
from
eba7cfc to
4f8a506
Compare
| @@ -231,6 +249,7 @@ run_pip_command() | |||
| ([ "$para" == "-c" ] || [ "$para" == "--constraint" ]) && found=y | |||
| if [ "$para" == "install" ]; then | |||
| install=y | |||
| parameters[${count}]="install " | |||
There was a problem hiding this comment.
build failed:
#20 [16/73] RUN pip3 install --upgrade pip
#20 0.829 ERROR: unknown command "install " - maybe you meant "install"
#20 ERROR: process "/bin/sh -c pip3 install --upgrade pip" did not complete successfully: exit code: 1
There was a problem hiding this comment.
I didn't hit this issue in my local build. Looks like some ENV setting is causing the issue.
Expand as :
"install " => install ( local build)
"install " => "install " ( community build)
Anyway, I have fixed it by removing the quotes.
During build, lots of pip packages are getting installed through pip install command. This feature adds support for caching all the pip packages into local cache path, so that subsequent build always loads from the cache.
Kalimuthu-Velappan
force-pushed
the
VERSION_CACHE_PIP
branch
from
4f8a506 to
050b35f
Compare
b) docker-sonic-mgmt-framework.gz.log If it's working for you then we can merge this PR and I'll just retest this later and provide feedback then. For additional information how do I test this:
|
During build, lots of pip packages are getting installed through pip install command. This feature adds support for caching all the pip packages into local cache path, so that subsequent build always loads from the cache.
During build, lots of pip packages are getting installed through pip install command.
This feature adds support for caching all the pip packages into local cache path, so that subsequent build always loads from the cache.
Why I did it
How I did it
How to verify it
Which release branch to backport (provide reason below if selected)
Description for the changelog
Ensure to add label/tag for the feature raised. example - PR#2174 under sonic-utilities repo. where, Generic Config and Update feature has been labelled as GCU.
Link to config_db schema for YANG module changes
A picture of a cute animal (not mandatory but encouraged)