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.

Sign up for GitHub

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

Jump to bottom

Fix key missing exception when invalied transiver info in STATE_DB #289

Merged
merged 3 commits into from
Aug 31, 2023
Merged

Fix key missing exception when invalied transiver info in STATE_DB #289

merged 3 commits into from
Aug 31, 2023

Conversation

liuh-80
Copy link
Contributor

@liuh-80 liuh-80 commented Aug 23, 2023

Fix PhysicalSensorTableMIBUpdater crash when 'type' attribute missing.

Work item tracking

Microsoft ADO (number only): 24869277

- What I did
Fix PhysicalSensorTableMIBUpdater crash when 'type' attribute missing.

- How I did it
Check 'type' attribute, if missing write error log in PhysicalSensorTableMIBUpdater.

- How to verify it
Manually test.
Pass all UT

- Description for the changelog
Fix PhysicalSensorTableMIBUpdater crash when 'type' attribute missing.

@liuh-80 liuh-80 marked this pull request as ready for review August 23, 2023 02:48
qiluo-msft
qiluo-msft reviewed Aug 29, 2023
View reviewed changes
src/sonic_ax_impl/mibs/ietf/rfc3433.py Outdated
@@ -424,6 +425,16 @@ def update_xcvr_dom_data(self):

# skip RJ45 port
Copy link
Contributor

@qiluo-msft qiluo-msft Aug 29, 2023 •
edited
Loading

Choose a reason for hiding this comment

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

skip RJ45 port

Better to move comments near the line of RJ45_PORT_TYPE. #Closed

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Fixed

qiluo-msft
qiluo-msft reviewed Aug 29, 2023
View reviewed changes
src/sonic_ax_impl/mibs/ietf/rfc3433.py
mibs.logger.error(
"Invalid interface {} in STATE_DB, \
attribute 'type' missing in transceiver_info '{}'".format(interface, transceiver_info_entry_data))
self.broken_transceiver_info.append(interface)
Copy link
Contributor

@qiluo-msft qiluo-msft Aug 29, 2023 •
edited
Loading

Choose a reason for hiding this comment

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

broken_transceiver_info

Not an issue and just comment: since the broken_transceiver_info is indexed by interface, and interface is validated above, so there are limited number of possible interface, so the broken_transceiver_info will have size limitation. #Closed

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Fixed by flush broken_transceiver_info in reinit_data()

Copy link
Contributor

Choose a reason for hiding this comment

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

Sorry for misleading, my point is this is not an issue or a bug. Because broken_transceiver_info has limited size by its current usage. No need to reinit.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Unnecessary change reverted.

@qiluo-msft qiluo-msft merged commit e3bcffe into sonic-net:master Aug 31, 2023
5 checks passed
@ZhaohuiS
Copy link

@liuh-80 Does this change need to be cherry picked into 202012, 202205, 202305?

@StormLiangMS
Copy link

@liuh-80 @qiluo-msft do we also need this on 202012?

@qiluo-msft
Copy link
Contributor

No such bug on 202111 or earlier branches. This PR is needed on 202205, 202305, but these branches are not existing right now.

@qiluo-msft qiluo-msft mentioned this pull request Sep 16, 2023
Closed
11 tasks
@liuh-80
Copy link
Contributor Author

liuh-80 commented Sep 18, 2023

@liuh-80 @qiluo-msft do we also need this on 202012?

We don't need this on 202012 because the code which has issue does not exist on 202012.

yxieca pushed a commit that referenced this pull request Oct 13, 2023
@liuh-80 @yxieca
Fix key missing exception when invalied transiver info in STATE_DB (#289
b5a52ff 
)

Fix PhysicalSensorTableMIBUpdater crash when 'type' attribute missing.

#### Work item tracking
Microsoft ADO (number only): 24869277

**- What I did**
Fix PhysicalSensorTableMIBUpdater crash when 'type' attribute missing.

**- How I did it**
Check 'type' attribute, if missing write error log in PhysicalSensorTableMIBUpdater.

**- How to verify it**
Manually test.
Pass all UT

**- Description for the changelog**
Fix PhysicalSensorTableMIBUpdater crash when 'type' attribute missing.
StormLiangMS pushed a commit that referenced this pull request Oct 17, 2023
@liuh-80 @StormLiangMS
Fix key missing exception when invalied transiver info in STATE_DB (#289
ead6e0f 
)

Fix PhysicalSensorTableMIBUpdater crash when 'type' attribute missing.

#### Work item tracking
Microsoft ADO (number only): 24869277

**- What I did**
Fix PhysicalSensorTableMIBUpdater crash when 'type' attribute missing.

**- How I did it**
Check 'type' attribute, if missing write error log in PhysicalSensorTableMIBUpdater.

**- How to verify it**
Manually test.
Pass all UT

**- Description for the changelog**
Fix PhysicalSensorTableMIBUpdater crash when 'type' attribute missing.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@qiluo-msft qiluo-msft qiluo-msft approved these changes

@keboliu keboliu Awaiting requested review from keboliu

Assignees
No one assigned
Labels
Included in 202205 Branch Included in 202305 Branch Request for 202205 Branch Request for 202305 Branch
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
5 participants
@liuh-80 @ZhaohuiS @StormLiangMS @qiluo-msft @yxieca