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

[sonic-yang-mgmt] Build PY3 & PY2 packages #5559

Merged
merged 10 commits into from
Nov 7, 2020

Conversation

praveen-li
Copy link
Member

@praveen-li praveen-li commented Oct 7, 2020

Changes:
-- change dep and mk files in rules for PY2.
-- add dep and mk files in rules for PY3.

Signed-off-by: Praveen Chaudhary[email protected]

- Why I did it
Moving sonic-yang-mgmt to PY3 to support move of sonic-utilities to PY3.

- How I did it

- How to verify it
Build time Test and VS tests must pass with new PKG.

- Which release branch to backport (provide reason below if selected)

  • 201811
  • 201911
  • 202006

- Description for the changelog

- A picture of a cute animal (not mandatory but encouraged)

@praveen-li praveen-li force-pushed the sonic-yang-mgmt-py3 branch from 0021196 to 1aa11fe Compare October 7, 2020 21:44
@lguohan lguohan requested a review from jleveque October 8, 2020 13:27
@jleveque
Copy link
Contributor

jleveque commented Oct 8, 2020

@praveen-li: There are build failures. Can you check? Also, it appears you are no longer building a Python 2 version of the package. Will the Python 2 version of sonic-utilities continue to work until I finish converting it to Python 3?

@praveen-li
Copy link
Member Author

retest vs please

@praveen-li
Copy link
Member Author

@jleveque

I think, to support test run in sonic-slave docker, we need these PKGs there. Because below errors are seen, even when tests_require is added.

06:55:25  Ran 0 tests in 0.000s
06:55:25  
06:55:25  OK
06:55:25  running bdist_wheel
06:55:25  running build
06:55:25  running build_py
06:55:25  Traceback (most recent call last):
06:55:25    File "./tests/yang_model_tests/yangModelTesting.py", line 7, in <module>
06:55:25      import ijson
06:55:25  ImportError: No module named ijson
06:55:25  YANG Tests failed

@jleveque
Copy link
Contributor

From the build output, I don't see any attempt from setuptools to install the package. Also, it shows that zero tests were run. Is this expected? If not, it seems like there may be other issues with your setup.py file.

@praveen-li
Copy link
Member Author

From the build output, I don't see any attempt from setuptools to install the package. Also, it shows that zero tests were run. Is this expected? If not, it seems like there may be other issues with your setup.py file.

[PC]: Zero tests are expected due to import error. But let me check because I noticed an attempt to install PKGs in earlier builds.

@praveen-li
Copy link
Member Author

From my build, I see an attempt to install the PKGs, I feel it is done because of install_require part.
But then still the tests are failing while importing those packages

Searching for ijson==2.6.1
Best match: ijson 2.6.1
Processing ijson-2.6.1-py2.7-linux-x86_64.egg

Using /sonic/src/sonic-yang-mgmt/.eggs/ijson-2.6.1-py2.7-linux-x86_64.egg
Searching for xmltodict==0.12.0
Best match: xmltodict 0.12.0
Processing xmltodict-0.12.0-py2.7.egg

Using /sonic/src/sonic-yang-mgmt/.eggs/xmltodict-0.12.0-py2.7.egg
running egg_info
writing requirements to sonic_yang_mgmt.egg-info/requires.txt
writing sonic_yang_mgmt.egg-info/PKG-INFO
writing top-level names to sonic_yang_mgmt.egg-info/top_level.txt
writing dependency_links to sonic_yang_mgmt.egg-info/dependency_links.txt
reading manifest file 'sonic_yang_mgmt.egg-info/SOURCES.txt'
writing manifest file 'sonic_yang_mgmt.egg-info/SOURCES.txt'
running build_ext
----------------------------------------------------------------------
Ran 0 tests in 0.000s

OK
/usr/lib/python2.7/distutils/dist.py:267: UserWarning: Unknown distribution option: 'setup_requirements'
  warnings.warn(msg)
running bdist_wheel
running build
running build_py
  =========================== test session starts ==============================
platform linux2 -- Python 2.7.16, pytest-3.10.1, py-1.7.0, pluggy-0.8.0
rootdir: /sonic/src/sonic-yang-mgmt, inifile:
plugins: cov-2.6.0
collected 1 item / 1 errors

==================================== ERRORS ====================================
________ ERROR collecting tests/libyang-python-tests/test_sonic_yang.py ________
ImportError while importing test module '/sonic/src/sonic-yang-mgmt/tests/libyang-python-tests/test_sonic_yang.py'.
Hint: make sure your test modules/packages have valid Python names.
Traceback:
/usr/lib/python2.7/dist-packages/six.py:709: in exec_
    exec("""exec _code_ in _globs_, _locs_""")
tests/libyang-python-tests/test_sonic_yang.py:4: in <module>
    import sonic_yang as sy
sonic_yang.py:6: in <module>
    from sonic_yang_ext import SonicYangExtMixin, SonicYangException
sonic_yang_ext.py:10: in <module>
    from xmltodict import parse
E   ImportError: No module named xmltodict
!!!!!!!!!!!!!!!!!!! Interrupted: 1 errors during collection !!!!!!!!!!!!!!!!!!!!
=========================== 1 error in 0.25 seconds ============================

src/sonic-yang-models/setup.py Outdated Show resolved Hide resolved
@jleveque
Copy link
Contributor

@praveen-li: Your build output is different from the check build logs here. I'm not sure why. I don't understand what's going on here, but it appears the dependencies aren't getting installed properly. I haven't encountered this issue with other packages.

@jleveque
Copy link
Contributor

Your setup.py file seems to be a bit more complex than others in the SONiC codebase (like sonic-utilities). Maybe the issue has something to do with the additional functions you define, like pkgBuild()?

@renukamanavalan renukamanavalan self-requested a review October 23, 2020 14:10
@praveen-li praveen-li changed the title [sonic-yang-mgmt-py3]: Build PY3 PKG from sonic-yang-mgmt instead of … [sonic-yang-mgmt-py3]: Build PY3 & PY2 PKGs for sonic-yang-mgmt. Oct 23, 2020
@praveen-li praveen-li force-pushed the sonic-yang-mgmt-py3 branch 2 times, most recently from b831ea9 to 46f1e6b Compare October 23, 2020 23:58
@lgtm-com
Copy link

lgtm-com bot commented Oct 24, 2020

This pull request introduces 2 alerts and fixes 1 when merging 46f1e6b809ba9a672b805607f3b6b2ee2fa10865 into 67408c8 - view on LGTM.com

new alerts:

  • 2 for Unused import

fixed alerts:

  • 1 for Unused import

src/sonic-yang-models/setup.py Outdated Show resolved Hide resolved
src/sonic-yang-mgmt/setup.py Show resolved Hide resolved
@lgtm-com
Copy link

lgtm-com bot commented Oct 26, 2020

This pull request introduces 2 alerts and fixes 1 when merging a4f359d0aeda52f5a7dc1b01833da2f7695741e4 into 9e34003 - view on LGTM.com

new alerts:

  • 2 for Unused import

fixed alerts:

  • 1 for Unused import

@lgtm-com
Copy link

lgtm-com bot commented Oct 26, 2020

This pull request fixes 1 alert when merging f43400274e07b3596fda178c7e073018d2ab3a49 into 9e34003 - view on LGTM.com

fixed alerts:

  • 1 for Unused import

@lgtm-com
Copy link

lgtm-com bot commented Oct 27, 2020

This pull request fixes 1 alert when merging 30d76ec1550d8fc5c43abaef1fed1e6bcb6e3f6d into dfe0055 - view on LGTM.com

fixed alerts:

  • 1 for Unused import

@praveen-li
Copy link
Member Author

retest mellanox please

@lgtm-com
Copy link

lgtm-com bot commented Oct 28, 2020

This pull request fixes 1 alert when merging b54e9dc8a2beccb7b5064dd93dab58b7e64cd716 into 09d5a62 - view on LGTM.com

fixed alerts:

  • 1 for Unused import

@jleveque
Copy link
Contributor

Retest broadcom please

@jleveque
Copy link
Contributor

Retest mellanox please

@jleveque
Copy link
Contributor

Retest vsimage please

@lguohan
Copy link
Collaborator

lguohan commented Nov 4, 2020

retest vsimage please

lguohan
lguohan previously approved these changes Nov 4, 2020
@lguohan
Copy link
Collaborator

lguohan commented Nov 5, 2020

retest vsimage please

Praveen Chaudhary added 10 commits November 5, 2020 13:02
…-mgmt, and fix VS build.

Signed-off-by: Praveen Chaudhary<[email protected]>
…n sonic-slave.

Install Pyang 2.1.1 using pip2 in sonic-slave, because we use it as pyang command,
and pip2 2.1.1 is stable with sonic yang models

Signed-off-by: Praveen Chaudhary<[email protected]>
… resolved conflicts.

Signed-off-by: Praveen Chaudhary<[email protected]>
@lgtm-com
Copy link

lgtm-com bot commented Nov 5, 2020

This pull request fixes 1 alert when merging 9207b2f into 13ff7b3 - view on LGTM.com

fixed alerts:

  • 1 for Unused import

@lguohan
Copy link
Collaborator

lguohan commented Nov 6, 2020

retest broadcom please

@lguohan
Copy link
Collaborator

lguohan commented Nov 7, 2020

@jleveque, can you check this pr? everything passed.

@praveen-li
Copy link
Member Author

@jleveque let me know if anything pending on me, thanks.

@jleveque jleveque changed the title [sonic-yang-mgmt-py3]: Build PY3 & PY2 PKGs for sonic-yang-mgmt. [sonic-yang-mgmt] Build PY3 & PY2 packages Nov 7, 2020
@jleveque jleveque merged commit 6156cb2 into sonic-net:master Nov 7, 2020
santhosh-kt pushed a commit to santhosh-kt/sonic-buildimage that referenced this pull request Feb 25, 2021
Moving sonic-yang-mgmt to PY3 to support move of sonic-utilities to PY3.

Signed-off-by: Praveen Chaudhary<[email protected]>
@qiluo-msft
Copy link
Collaborator

qiluo-msft commented Oct 20, 2021

                     './yang-models/sonic_yang_tree']),

This file is generated by test, not part of source code.
This PR breaks pip3 install . if the file is not existing.


In reply to: 947590563


Refers to: src/sonic-yang-models/setup.py:57 in 9207b2f. [](commit_id = 9207b2f, deletion_comment = False)

@qiluo-msft
Copy link
Collaborator

                     './yang-models/sonic_yang_tree']),

How critical is this file? I think it is just for document purpose. A quick way to unblock the command is to remvoe it.


In reply to: 947590563


Refers to: src/sonic-yang-models/setup.py:57 in 9207b2f. [](commit_id = 9207b2f, deletion_comment = False)

@praveen-li
Copy link
Member Author

                     './yang-models/sonic_yang_tree']),

This file is generated by test, not part of source code. This PR breaks pip3 install . if the file is not existing.

Refers to: src/sonic-yang-models/setup.py:57 in 9207b2f. [](commit_id = 9207b2f, deletion_comment = False)

Hi Qi,

This file is auto-generated by test. We made it part of test, because it also verified that all syntax and hierarchy in yang models are correct.
We need this file, if It is not generated then YANG models are not correct and will fail at runtime. Thx.

@qiluo-msft
Copy link
Collaborator

                     './yang-models/sonic_yang_tree']),

Offline discussed. We could ondemand get the content by

pyang -Vf tree -p /usr/local/share/yang/modules/ietf /usr/local/yang-models/*.yang

It is not a critical content in the package, so we can safely remove it.


In reply to: 947591498


Refers to: src/sonic-yang-models/setup.py:57 in 9207b2f. [](commit_id = 9207b2f, deletion_comment = False)

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

Successfully merging this pull request may close these issues.

5 participants