Skip to content
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

Click Upgrade to 7.0.0 #3907

Closed
wants to merge 1 commit into from
Closed

Conversation

samaity
Copy link
Collaborator

@samaity samaity commented Dec 14, 2019

Signed-off-by: Sangita Maity sangitamaity0211@gmail.com

- What I did
Upgraded click to 7.0.0 via pip. This is needed to support autocompletion part in breakout cli command to support dynamic option for mode choice.

- How to verify it

Built python-sonic-utilities_1.2-1_all.deb package successfully.

samaity@b7f80e98606a:/sonic$ ls -la target/python-debs/python-sonic-utilities_1.2-1_all.deb*
-rw-r--r-- 1 samaity gsamaity 120814 Dec 13 07:16 target/python-debs/python-sonic-utilities_1.2-1_all.deb
-rw-r--r-- 1 samaity gsamaity  76581 Dec 13 07:16 target/python-debs/python-sonic-utilities_1.2-1_all.deb.log
samaity@b7f80e98606a:/sonic$

samaity@b7f80e98606a:/sonic$ BLDENV=stretch make -f slave.mk target/python-debs/python-sonic-utilities_1.2-1_all.deb-clean
SONiC Build System

Build Configuration
"CONFIGURED_PLATFORM"             : "vs"
"CONFIGURED_ARCH"                 : "amd64"
"SONIC_CONFIG_PRINT_DEPENDENCIES" : ""
"SONIC_BUILD_JOBS"                : "1"
"SONIC_CONFIG_MAKE_JOBS"          : "24"
"SONIC_USE_DOCKER_BUILDKIT"       : ""
"USERNAME"                        : "admin"
"PASSWORD"                        : "admin123"
"ENABLE_DHCP_GRAPH_SERVICE"       : ""
"SHUTDOWN_BGP_ON_START"           : ""
"ENABLE_PFCWD_ON_START"           : ""
"INSTALL_DEBUG_TOOLS"             : ""
"ROUTING_STACK"                   : "frr"
"FRR_USER_UID"                    : "300"
"FRR_USER_GID"                    : "300"
"ENABLE_SYNCD_RPC"                : ""
"ENABLE_ORGANIZATION_EXTENSIONS"  : "y"
"HTTP_PROXY"                      : ""
"HTTPS_PROXY"                     : ""
"ENABLE_SYSTEM_TELEMETRY"         : "y"
"SONIC_DEBUGGING_ON"              : ""
"SONIC_PROFILING_ON"              : ""
"KERNEL_PROCURE_METHOD"           : "build"
"BUILD_TIMESTAMP"                 : ""
"BLDENV"                          : "stretch"
"VS_PREPARE_MEM"                  : "yes"
"ENABLE_SFLOW"                    : "y"

built python-sonic-utilities_1.2-1_all.deb package successfully.

samaity@b7f80e98606a:/sonic$ BLDENV=stretch make -f slave.mk target/python-debs/python-sonic-utilities_1.2-1_all.deb
SONiC Build System

Build Configuration
"CONFIGURED_PLATFORM"             : "vs"
"CONFIGURED_ARCH"                 : "amd64"
"SONIC_CONFIG_PRINT_DEPENDENCIES" : ""
"SONIC_BUILD_JOBS"                : "1"
"SONIC_CONFIG_MAKE_JOBS"          : "24"
"SONIC_USE_DOCKER_BUILDKIT"       : ""
"USERNAME"                        : "admin"
"PASSWORD"                        : "admin123"
"ENABLE_DHCP_GRAPH_SERVICE"       : ""
"SHUTDOWN_BGP_ON_START"           : ""
"ENABLE_PFCWD_ON_START"           : ""
"INSTALL_DEBUG_TOOLS"             : ""
"ROUTING_STACK"                   : "frr"
"FRR_USER_UID"                    : "300"
"FRR_USER_GID"                    : "300"
"ENABLE_SYNCD_RPC"                : ""
"ENABLE_ORGANIZATION_EXTENSIONS"  : "y"
"HTTP_PROXY"                      : ""
"HTTPS_PROXY"                     : ""
"ENABLE_SYSTEM_TELEMETRY"         : "y"
"SONIC_DEBUGGING_ON"              : ""
"SONIC_PROFILING_ON"              : ""
"KERNEL_PROCURE_METHOD"           : "build"
"BUILD_TIMESTAMP"                 : ""
"BLDENV"                          : "stretch"
"VS_PREPARE_MEM"                  : "yes"
"ENABLE_SFLOW"                    : "y"

samaity@b7f80e98606a:/sonic$

checked click version in newly built sonic-slave docker.

samaity@b7f80e98606a:/sonic$ pip list | grep click
DEPRECATION: The default format will switch to columns in the future. You can use --format=(legacy|columns) (or define a format=(legacy|columns) in your pip.conf under the [list] section) to disable this warning.
click-default-group (1.2.2)
samaity@b7f80e98606a:/sonic$ pip list | grep Click
DEPRECATION: The default format will switch to columns in the future. You can use --format=(legacy|columns) (or define a format=(legacy|columns) in your pip.conf under the [list] section) to disable this warning.
Click (7.0)
samaity@b7f80e98606a:/sonic$

Signed-off-by: Sangita Maity <sangitamaity0211@gmail.com>
@samaity samaity marked this pull request as ready for review December 14, 2019 20:52
@jleveque
Copy link
Contributor

jleveque commented Dec 16, 2019

@samaity: With this solution, do we wind up with two versions of Click installed on the system (a older version via apt-get, the this one via pip)? The issue is that sonic-utilities is build as a .deb package, so it attempts to install its dependencies as .deb files. This was the reason why I opted to build the .deb file from source. See #1824

@sonic-net sonic-net deleted a comment from lguohan Dec 18, 2019
@jleveque
Copy link
Contributor

retest vsimage please

@jleveque
Copy link
Contributor

Retest vsimage please

@samaity
Copy link
Collaborator Author

samaity commented Jan 30, 2020

@samaity: With this solution, do we wind up with two versions of Click installed on the system (a older version via apt-get, the this one via pip)? The issue is that sonic-utilities is build as a .deb package, so it attempts to install its dependencies as .deb files. This was the reason why I opted to build the .deb file from source. See #1824

@jleveque, As far as I see using pip list command, only one Click version 7.0 got installed. Is there any other place where I should check the click version?

admin@lnos-x1-a-asw01:~$ pip list
DEPRECATION: Python 2.7 reached the end of its life on January 1st, 2020. Please upgrade your Python as Python 2.7 is no longer maintained. A future version of pip will drop support for Python 2.7. More details about Python 2 support in pip, can be found at https://pip.pypa.io/en/latest/development/release-process/#python-2-support
Package               Version
--------------------- ---------------
argcomplete           1.8.1
bitarray              0.8.1
certifi               2019.11.28
chardet               3.0.4
Click                 7.0
click-default-group   1.2
colorama              0.3.7
docker-py             1.6.0
idna                  2.8
ipaddr                2.1.11
ipaddress             1.0.23
Jinja2                2.10.3
jsondiff              1.2.0
lxml                  3.7.1
MarkupSafe            1.1.1
natsort               4.0.3
netaddr               0.7.19
netifaces             0.10.7
pexpect               4.8.0
pip                   20.0.1
psutil                5.6.7
ptyprocess            0.6.0
pyang                 2.1.1
pyangbind             0.6.0
python-apt            1.4.1
PyYAML                3.12
redis                 2.10.6
redis-dump-load       1.1
requests              2.22.0
scapy                 unknown.version
setuptools            33.1.1
six                   1.14.0
sonic-config-engine   1.0
sonic-daemon-base     1.0
sonic-platform        1.0
sonic-platform-common 1.0
sonic-utilities       1.2
sonic-yang-mgmt       1.0
swsssdk               2.0.1
tabulate              0.8.2
urllib3               1.25.8
websocket-client      0.57.0
xmltodict             0.12.0

@jleveque
Copy link
Contributor

Is there any other place where I should check the click version?

Yes. Run dpkg -l

@lguohan
Copy link
Collaborator

lguohan commented Feb 3, 2020

the easiest change is to something similar to this one.

lguohan@e6d7455

@samaity
Copy link
Collaborator Author

samaity commented Feb 3, 2020

Is there any other place where I should check the click version?

Yes. Run dpkg -l

Yes @jleveque, you are right. I have another version seating there.

@samaity
Copy link
Collaborator Author

samaity commented Feb 3, 2020

the easiest change is to something similar to this one.

lguohan@e6d7455

yes, I am testing it out. As far as I remember, I tried to do something similar to this, but I was experiencing some problems build the deb package. will update once I am done with this. Thanks, @lguohan !!!

@stevenlu99
Copy link
Contributor

Do we have updates on this pr?

@samaity
Copy link
Collaborator Author

samaity commented Feb 26, 2020

Hi @jleveque @lguohan @stevenlu99,
I tried to create the Debian package for click 7.0-1 how @lguohan suggested or @jleveque did for click 6.7-4. But I am not able to do it successfully as there are too many dependencies for click and it's subsequent dependent packages. The problem is mainly due to the Debian stretch that does not have those dependencies available on how Debian buster has.

python-click is dependent on python-pallets-sphinx-themes which is not available to Debian stretch. Check out the link So, I cant use apt-get to get this package. I tried the to build python-pallets-sphinx-themes deb package but this package again dependent on few more packages which are not available to Debian stretch i.e. python3-importlib-metadata and many more and the story goes on and on.

My suggestion would be to remove the install requirement of click from sonic-utilities setup.py file. As we are already installing click via pip and once, we move the dependency requirement, sonic-utilities is built as a .deb package, it will not attempt to install click once again. So, we will not have two versions of Click installed on the system.

Let me know how it sounds.

@jleveque
Copy link
Contributor

jleveque commented Feb 26, 2020

@samaity: This brings us back to a conundrum we have faced a few times in the past. Ultimately, we would like to properly configure dependencies for all of our packages. However, if we build a Python package as a .deb package, it looks for its dependencies as other installed .deb packages. The opposite goes for building as a .whl -- it will look for its dependencies as installed .whls.

I tried building sonic-utilities as a .whl in the past -- this way we could install all Python packages as .whls. However, .whl packages are not allowed to install files outside of the /usr/local/lib/python2.7/dist-packages/ directory, which is something we do with this package.

So, we can do as you propose (install Click as a .whl and remove it from the sonic-utilities dependencies), but we lose the intrinsic check for dependencies (I must admit, we have already done this for a couple other dependencies, which I have tried to make clear in the setup.py file). It's not ideal, but we could make it work.

A bigger, separate concern for me is that currently we are building Click 6.7-4 from source because I added some patches to fix some bugs. The Click folks have since accepted and merged my patches in their upstream codebase. First, we need to confirm that my fixes made it into Click 7.0. If so, that's great. If not, we will need to continue building from source; in which case, we will have the same dependency issues you encountered until we move to Buster.

@samaity
Copy link
Collaborator Author

samaity commented Feb 26, 2020

Hey @jleveque , I checked the Click 7.0 code. I see the fixes incorporated there so we won't get any bug regarding CLI autocomplete/suggest. So, we are good there.

I have removed click from install_requirements and once we move to debian buster , we can build it as deb package as @lguohan suggested i.e. lguohan@e6d7455.

@lguohan
Copy link
Collaborator

lguohan commented Mar 6, 2020

@samaity , can you change your pr to build the deb package?

@lguohan
Copy link
Collaborator

lguohan commented Apr 23, 2020

obseleted by 4e07c78

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants