Skip to content

[Platform API][PSU][Component] Compare applicable data against values from platform.json#2267

Merged
jleveque merged 5 commits intosonic-net:masterfrom
sujinmkang:p_test
Sep 25, 2020
Merged

[Platform API][PSU][Component] Compare applicable data against values from platform.json#2267
jleveque merged 5 commits intosonic-net:masterfrom
sujinmkang:p_test

Conversation

@sujinmkang
Copy link
Copy Markdown
Contributor

Description of PR

Summary:
Fixes # (issue)

Type of change

  • Bug fix
  • Testbed and Framework(new/improvement)
  • Test case(new/improvement)

Approach

What is the motivation for this PR?

How did you do it?

How did you verify/test it?

Any platform specific information?

Supported testbed topology if it's a new test case?

Documentation

Copy link
Copy Markdown
Collaborator

@yxieca yxieca left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

My only concern is that these warnings will be ignored. Had a discussion with Joe offline regarding this on another PR.

I think we could add a specific test to test that platform.json exists and that test can fail and indicating work needed. If the test could further validate required fields and failing on them too, then these warnings are fine.

@jleveque
Copy link
Copy Markdown
Contributor

jleveque commented Sep 25, 2020

@yxieca: I am now of the belief that we should simply consider the platform.json file required, and fail any tests which rely on its data if it's not available. This will result in more consistency across platforms. When we first designed the tests, this data was more of an "enhanced" version of the tests, but I believe it should now be mandatory.

Here is a PR for the chassis and SFP tests: #2268

@jleveque jleveque changed the title [Platform API][PSU&COMPONENT]compare applicable data against values from platform.json [Platform API][PSU][Component] Compare applicable data against values from platform.json Sep 25, 2020
@lgtm-com
Copy link
Copy Markdown

lgtm-com bot commented Sep 25, 2020

This pull request fixes 1 alert when merging 872e72f into d83d544 - view on LGTM.com

fixed alerts:

  • 1 for Unused import

@lgtm-com
Copy link
Copy Markdown

lgtm-com bot commented Sep 25, 2020

This pull request introduces 2 alerts and fixes 8 when merging 026a5da into 30180c9 - view on LGTM.com

new alerts:

  • 2 for Syntax error

fixed alerts:

  • 5 for Unused import
  • 3 for Except block handles 'BaseException'

@jleveque jleveque merged commit dddaefa into sonic-net:master Sep 25, 2020
kazinator-arista pushed a commit to kazinator-arista/sonic-mgmt that referenced this pull request Mar 4, 2026
swss

081d47b Update netlink messages handler (sonic-net#2233)
de7c3eb [IntfMgrd] Retry adding ipv6 prefix by setting disabled_ipv6 flag  (sonic-net#2267)

utilities

2716ff2 [yang] extend ConfigMgmt constructor to pass YANG options (sonic-net#2118)
9fbe2ef [debug dump] dump interface module added (sonic-net#2070)
a86da2d Add sonic-delayed.target to Application Extension .timer file generator (sonic-net#2176)
90611dd [portconfig] Allow to configure interface mtu for physical ports only
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.

3 participants