[sonic-py-common] Add platform.json to port_config files candidates only if interfaces are present within platform.json #5538
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Signed-off-by: vaibhav-dahiya <vdahiya@microsoft.com>
jleveque
suggested changes
Oct 5, 2020
jleveque
changed the title
Signed-off-by: vaibhav-dahiya <vdahiya@microsoft.com>
jleveque
suggested changes
Oct 8, 2020
| if os.path.isfile(os.path.join(platform_path, PLATFORM_JSON_FILE)): | ||
| json_file = os.path.join(platform_path, PLATFORM_JSON_FILE) | ||
| platform_data = json.loads(open(json_file).read()) | ||
| if len(platform_data.get("interfaces")) > 0: |
There was a problem hiding this comment.
Maybe you should also first check for the presence of the "interfaces" key. That way, if either the key is not present OR it is empty, we don't consider platform.json a candidate.
Signed-off-by: vaibhav-dahiya <vdahiya@microsoft.com>
This reverts commit 166603d.
Signed-off-by: vaibhav-dahiya <vdahiya@microsoft.com>
Signed-off-by: vaibhav-dahiya <vdahiya@microsoft.com>
|
Retest this please |
|
Retest mellanox please |
Signed-off-by: vaibhav-dahiya <vdahiya@microsoft.com>
jleveque
suggested changes
Oct 12, 2020
Comment on lines
272
to
273
| if interfaces: | ||
| if len(interfaces) > 0: |
There was a problem hiding this comment.
I believe if interfaces: should be enough, as it will return false if either interfaces is None or its length is 0. If you want to be more explicit, I recommend if interfaces is not None and len(interfaces) > 0:
Signed-off-by: vaibhav-dahiya <vdahiya@microsoft.com>
jleveque
approved these changes
Oct 12, 2020
|
Retest this please |
|
Retest vsimage please |
1 similar comment
|
Retest vsimage please |
santhosh-kt
pushed a commit
to santhosh-kt/sonic-buildimage
that referenced
this pull request
Feb 25, 2021
…nly if interfaces are present within platform.json (sonic-net#5538) * [sonic-py-common] add platform.json to port_config Signed-off-by: vaibhav-dahiya <vdahiya@microsoft.com>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Summary:
For this PR the motivation is to add logic so that platform.json is considered for port configuration candidates only if platform.json has an interfaces key
and the length of the key is greater than zero. This happened with new platform.json file utilized to test platform_api's infrastructure which in turn had interfaces key without any port configs.
Type of change
Approach
What is the motivation for this PR?
To make sure platform.json is not used for sonic-cfggen if it does not have "interfaces" key populated
How did you do it?
added logic to check if platform.json is present, check if "interfaces" key size is greater than zero
How did you verify/test it?
Run it on a mellanox-acs2700 platform and verify cfggen does not cause errors
Any platform specific information?
platform.json should be present without "interfaces" key populated
Signed-off-by: vaibhav-dahiya vdahiya@microsoft.com