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-platform-daemons] submodule update #7143

Merged
merged 1 commit into from
Mar 30, 2021

Conversation

vdahiya12
Copy link
Contributor

@vdahiya12 vdahiya12 commented Mar 24, 2021

This PR updates the following commits in sonic-platform-daemons
260cf2d [xcvrd] change firmware information fields name inside MUX_CABLE_INFO table for Y cable (#165)
cfa600f [thermalctld] Initialize fan led in thermalctld for the first run (#167)
8509f43 [thermalctld] Refactor to allow for greater unit test coverage; Add more unit tests (#157)
70f4e7b [syseepromd] Update warning message to be more informative (#160)

Signed-off-by: vaibhav-dahiya [email protected]

Why I did it

How I did it

How to verify it

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

  • 201811
  • 201911
  • 202006
  • 202012

Description for the changelog

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

this PR updates the following commits in sonic-platform-daemons
260cf2d [xcvrd] change firmware information fields name inside MUX_CABLE_INFO table for Y cable (sonic-net#165)
cfa600f [thermalctld] Initialize fan led in thermalctld for the first run (sonic-net#167)
8509f43 [thermalctld] Refactor to allow for greater unit test coverage; Add more unit tests (sonic-net#157)
70f4e7b [syseepromd] Update warning message to be more informative (sonic-net#160)

Signed-off-by: vaibhav-dahiya <[email protected]>
@liat-grozovik
Copy link
Collaborator

/AzurePipelines run

@azure-pipelines
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@liat-grozovik
Copy link
Collaborator

/AzurePipelines run

@azure-pipelines
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@lguohan
Copy link
Collaborator

lguohan commented Mar 29, 2021

can you check the build failure?

@vdahiya12
Copy link
Contributor Author

in one of the commit sonic-thermalctld now added a dependency of sonic-platform-common in setup.py.
This is causing the build issue

[ FLAGS FILE ] : []
[ FLAGS DEPENDS ] : [broadcom]
[ FLAGS DIFF ] : [broadcom ]
/sonic/src/sonic-platform-daemons/sonic-thermalctld /sonic
running pytest
Searching for sonic-platform-common
Reading https://pypi.org/simple/sonic-platform-common/
Couldn't find index page for 'sonic-platform-common' (maybe misspelled?)
Scanning index of all packages (this may take a while)
Reading https://pypi.org/simple/
No local packages or working download links found for sonic-platform-common
error: Could not find suitable distribution for Requirement.parse('sonic-platform-common')
[ FAIL LOG END ] [ target/python-wheels/sonic_thermalctld-1.0-py2-none-any.whl ]
make: *** [slave.mk:600: target/python-wheels/sonic_thermalctld-1.0-py2-none-any.whl] Error 1
make: *** Waiting for unfinished jobs....
[ cached ] [ target/python-wheels/sonic_config_engine-1.0-py3-none-any.whl ]
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. pip 21.0 will drop support for Python 2.7 in January 2021. More details about Python 2 support in pip can be found at https://pip.pypa.io/en/latest/development/release-process/#python-2-support pip 21.0 will remove support for this functionality.
make[1]: *** [Makefile.work:265: target/sonic-broadcom.bin] Error 2
make[1]: Leaving directory '/agent/_work/1/s'
make: *** [Makefile:9: target/sonic-broadcom.bin] Error 2
##[error]Bash exited with code '2'.

@jleveque
Copy link
Contributor

@vdahiya12: That dependency was introduced by me when increasing unit test coverage. I have opened a PR to add the dependency in the makefile here: #7181

@dprital
Copy link
Collaborator

dprital commented Mar 30, 2021

The following (167) link on the description contains wrong link:
cfa600f [thermalctld] Initialize fan led in thermalctld for the first run (#167)

It should refer to the following PR: sonic-net/sonic-platform-daemons#167

@vdahiya12
Copy link
Contributor Author

The following (167) link on the description contains wrong link:
cfa600f [thermalctld] Initialize fan led in thermalctld for the first run (#167)

It should refer to the following PR: Azure/sonic-platform-daemons#167

Hi @dprital whenever we update the submodule pointer we always reference the commits with appropriate hash and PR # in the respective submodule repo. In this regard # 167 is actually correct, but you always have to go to the appropriate repo to get the correct link.
By default a copy paste of git log (HEAD)..master --oneline gives the output that is pasted above.
just that by default it always picks the PR number in the current repository, rather than the one in the submodule, which is incorrect, (but kind of the norm :-) ) and you have to follow the logic described above to get the correct PR in the submodule.
Hope it makes sense.

@jleveque
Copy link
Contributor

The following (167) link on the description contains wrong link:
cfa600f [thermalctld] Initialize fan led in thermalctld for the first run (#167)
It should refer to the following PR: Azure/sonic-platform-daemons#167

Hi @dprital whenever we update the submodule pointer we always reference the commits with appropriate hash and PR # in the respective submodule repo. In this regard # 167 is actually correct, but you always have to go to the appropriate repo to get the correct link.
By default a copy paste of git log (HEAD)..master --oneline gives the output that is pasted above.
just that by default it always picks the PR number in the current repository, rather than the one in the submodule, which is incorrect, (but kind of the norm :-) ) and you have to follow the logic described above to get the correct PR in the submodule.
Hope it makes sense.

This is how we've been approaching this thus far. It does technically cause GitHub to create links between unrelated PRs. Moving forward we could add the submodule prefix (e.g., Azure/sonic-platform-daemons) before each PR number in the description; it's just a bit time consuming.

@vdahiya12 vdahiya12 merged commit 4b2e83e into sonic-net:master Mar 30, 2021
daall pushed a commit that referenced this pull request Mar 31, 2021
this PR updates the following commits in sonic-platform-daemons
260cf2d [xcvrd] change firmware information fields name inside MUX_CABLE_INFO table for Y cable (#165)
cfa600f [thermalctld] Initialize fan led in thermalctld for the first run (#167)
8509f43 [thermalctld] Refactor to allow for greater unit test coverage; Add more unit tests (#157)
70f4e7b [syseepromd] Update warning message to be more informative (#160)

Signed-off-by: vaibhav-dahiya <[email protected]>
raphaelt-nvidia pushed a commit to raphaelt-nvidia/sonic-buildimage that referenced this pull request May 23, 2021
this PR updates the following commits in sonic-platform-daemons
260cf2d [xcvrd] change firmware information fields name inside MUX_CABLE_INFO table for Y cable (sonic-net#165)
cfa600f [thermalctld] Initialize fan led in thermalctld for the first run (sonic-net#167)
8509f43 [thermalctld] Refactor to allow for greater unit test coverage; Add more unit tests (sonic-net#157)
70f4e7b [syseepromd] Update warning message to be more informative (sonic-net#160)

Signed-off-by: vaibhav-dahiya <[email protected]>
carl-nokia pushed a commit to carl-nokia/sonic-buildimage that referenced this pull request Aug 7, 2021
this PR updates the following commits in sonic-platform-daemons
260cf2d [xcvrd] change firmware information fields name inside MUX_CABLE_INFO table for Y cable (sonic-net#165)
cfa600f [thermalctld] Initialize fan led in thermalctld for the first run (sonic-net#167)
8509f43 [thermalctld] Refactor to allow for greater unit test coverage; Add more unit tests (sonic-net#157)
70f4e7b [syseepromd] Update warning message to be more informative (sonic-net#160)

Signed-off-by: vaibhav-dahiya <[email protected]>
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.

6 participants