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

[multi_asic_host]: Add __repr__ method #3090

Merged
merged 3 commits into from
Mar 9, 2021

Conversation

theasianpianist
Copy link
Contributor

Signed-off-by: Lawrence Lee [email protected]

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?

When viewing stack traces for failed tests, any duthost objects are displayed using the default Python representation which simply gives the object's location in memory. On single DUT testbeds this is not an issue, but on dual ToR systems it is difficult to tell which DUT is relevant to the failure.

How did you do it?

Add the __repr__ method to the MultiAsicSonicHost class, which makes the class readable by using the device hostname as the representation.

This also gives the added benefit of returning the hostname when the duthost object is cast to a string (for example when formatting a string)

How did you verify/test it?

Run a test that failed. Confirm in the stack trace that the device hostname is shown instead of the host object's memory location.

Any platform specific information?

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

Documentation

@theasianpianist theasianpianist requested a review from a team March 4, 2021 06:48
@@ -43,6 +43,9 @@ def __init__(self, ansible_adhoc, hostname):

self.critical_services_tracking_list()

def __repr__(self):
return self.hostname
Copy link
Collaborator

Choose a reason for hiding this comment

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

Is it better to return something like '<MultiAsicSonicHost {}>'.format(self.hostname)?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Good idea, will change

@theasianpianist theasianpianist requested review from wangxin and a team March 8, 2021 22:34
@@ -43,6 +43,9 @@ def __init__(self, ansible_adhoc, hostname):

self.critical_services_tracking_list()

def __repr__(self):
return '<MultiAsicSonicHost> {}'.format(self.hostname)
Copy link
Contributor

Choose a reason for hiding this comment

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

can you fix the alignment?

@theasianpianist theasianpianist merged commit 2a7bc60 into sonic-net:master Mar 9, 2021
@theasianpianist theasianpianist deleted the duthost-repr branch March 9, 2021 18:37
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants