Outer try catch block in Parser updateVpdKeyword API #539
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
souvik1914581
pushed a commit
to souvik1914581/openpower-vpd-parser
that referenced
this pull request
Dec 12, 2024
This commit adds an outer try catch block in Parser::updateVpdKeyword API. Currently, this API has multiple return statements for different error scenarios, which reduces the API body's readability and modularity. In order to streamline and handle all errors in a single location, an outer try catch has been added in the API body. Test: ''' Trigger this API by calling WriteKeyword API from D-Bus: root@testhostname2:~# busctl call com.ibm.VPD.Manager /com/ibm/VPD/Manager com.ibm.VPD.Manager ReadKeyword sv "/sys/bus/i2c/drivers/at24/8-0050/eeprom" \(ss\) "UTIL" "D0" v ay 1 1 root@testhostname2:~# busctl call com.ibm.VPD.Manager /com/ibm/VPD/Manager com.ibm.VPD.Manager WriteKeyword sv "/sys/bus/i2c/drivers/at24/8-0050/eeprom" \(ssay\) "UTIL" "D0" 1 0 i 1 root@testhostname2:~# busctl call com.ibm.VPD.Manager /com/ibm/VPD/Manager com.ibm.VPD.Manager ReadKeyword sv "/sys/bus/i2c/drivers/at24/8-0050/eeprom" \(ss\) "UTIL" "D0" v ay 1 0 ''' Change-Id: I66a6c95ca0daaebb0a6f5c80e28ad4e9706d776f Signed-off-by: Souvik Roy <[email protected]>
souvik1914581
force-pushed
the
sr_addOuterTryCatchInUpdateVpdKeyword
branch
from
3d5407d to
8d22a83
Compare
souvik1914581
pushed a commit
to souvik1914581/openpower-vpd-parser
that referenced
this pull request
Dec 12, 2024
This commit adds an outer try catch block in Parser::updateVpdKeyword API. Currently, this API has multiple return statements for different error scenarios, which reduces the API body's readability and modularity. In order to streamline and handle all errors in a single location, an outer try catch has been added in the API body. This commit also adds constants to define success and failure for APIs which return 0 or -1. Test: ''' Trigger this API by calling WriteKeyword API from D-Bus: root@testhostname2:~# busctl call com.ibm.VPD.Manager /com/ibm/VPD/Manager com.ibm.VPD.Manager ReadKeyword sv "/sys/bus/i2c/drivers/at24/8-0050/eeprom" \(ss\) "UTIL" "D0" v ay 1 1 root@testhostname2:~# busctl call com.ibm.VPD.Manager /com/ibm/VPD/Manager com.ibm.VPD.Manager WriteKeyword sv "/sys/bus/i2c/drivers/at24/8-0050/eeprom" \(ssay\) "UTIL" "D0" 1 0 i 1 root@testhostname2:~# busctl call com.ibm.VPD.Manager /com/ibm/VPD/Manager com.ibm.VPD.Manager ReadKeyword sv "/sys/bus/i2c/drivers/at24/8-0050/eeprom" \(ss\) "UTIL" "D0" v ay 1 0 ''' Change-Id: I66a6c95ca0daaebb0a6f5c80e28ad4e9706d776f Signed-off-by: Souvik Roy <[email protected]>
souvik1914581
force-pushed
the
sr_addOuterTryCatchInUpdateVpdKeyword
branch
from
8d22a83 to
bf8679c
Compare
souvik1914581
requested review from
branupama,
SunnySrivastava1984,
PriyangaRamasamy,
jinuthomas and
RekhaAparna01
this is not the case. |
branupama
suggested changes
Dec 12, 2024
vpd-manager/src/parser.cpp
Outdated
|
|
||
| // TODO: Check if revert is required when any of the writes fails. | ||
| // TODO: Handle error logging | ||
| return constants::FAILURE; |
There was a problem hiding this comment.
we dont need this statement, as we will return at the end anyways. ?
There was a problem hiding this comment.
Suppose line 69 writeKeywordOnHardware is successful, l_bytesUpdatedOnHardware has some value > 0, but any subsequent steps like updateVpdKeywordOnRedundantPath fails, we have to return -1.
| if (updateVpdKeywordOnRedundantPath(l_redundantFruPath, | ||
| i_paramsToWriteData) < 0) | ||
| std::string l_keywordIdentifier{}; | ||
| if (const types::IpzData* l_ipzData = |
There was a problem hiding this comment.
as all the throw has sufficient information,
do we need extra log here, printing l_ex.what() is enough ?
There was a problem hiding this comment.
The throws do not log the Record Name and Keyword Name.
souvik1914581
pushed a commit
to souvik1914581/openpower-vpd-parser
that referenced
this pull request
Dec 13, 2024
This commit adds an outer try catch block in Parser::updateVpdKeyword API. Currently, this API has multiple return statements for different error scenarios, which reduces the API body's readability and modularity. In order to streamline and handle all errors in a single location, an outer try catch has been added in the API body. This commit also adds constants to define success and failure for APIs which return 0 or -1. Test: ''' Trigger this API by calling WriteKeyword API from D-Bus: root@testhostname2:~# busctl call com.ibm.VPD.Manager /com/ibm/VPD/Manager com.ibm.VPD.Manager ReadKeyword sv "/sys/bus/i2c/drivers/at24/8-0050/eeprom" \(ss\) "UTIL" "D0" v ay 1 1 root@testhostname2:~# busctl call com.ibm.VPD.Manager /com/ibm/VPD/Manager com.ibm.VPD.Manager WriteKeyword sv "/sys/bus/i2c/drivers/at24/8-0050/eeprom" \(ssay\) "UTIL" "D0" 1 0 i 1 root@testhostname2:~# busctl call com.ibm.VPD.Manager /com/ibm/VPD/Manager com.ibm.VPD.Manager ReadKeyword sv "/sys/bus/i2c/drivers/at24/8-0050/eeprom" \(ss\) "UTIL" "D0" v ay 1 0 ''' Change-Id: I66a6c95ca0daaebb0a6f5c80e28ad4e9706d776f Signed-off-by: Souvik Roy <[email protected]>
souvik1914581
force-pushed
the
sr_addOuterTryCatchInUpdateVpdKeyword
branch
from
bf8679c to
74b9822
Compare
This commit adds an outer try catch block in Parser::updateVpdKeyword API. Currently, this API has multiple return statements for different error scenarios, which reduces the API body's readability and modularity. In order to streamline and handle all errors in a single location, an outer try catch has been added in the API body. This commit also adds constants to define success and failure for APIs which return 0 or -1. Test: ''' Trigger this API by calling WriteKeyword API from D-Bus: root@testhostname2:~# busctl call com.ibm.VPD.Manager /com/ibm/VPD/Manager com.ibm.VPD.Manager ReadKeyword sv "/sys/bus/i2c/drivers/at24/8-0050/eeprom" \(ss\) "UTIL" "D0" v ay 1 1 root@testhostname2:~# busctl call com.ibm.VPD.Manager /com/ibm/VPD/Manager com.ibm.VPD.Manager WriteKeyword sv "/sys/bus/i2c/drivers/at24/8-0050/eeprom" \(ssay\) "UTIL" "D0" 1 0 i 1 root@testhostname2:~# busctl call com.ibm.VPD.Manager /com/ibm/VPD/Manager com.ibm.VPD.Manager ReadKeyword sv "/sys/bus/i2c/drivers/at24/8-0050/eeprom" \(ss\) "UTIL" "D0" v ay 1 0 ''' Change-Id: I66a6c95ca0daaebb0a6f5c80e28ad4e9706d776f Signed-off-by: Souvik Roy <[email protected]>
souvik1914581
force-pushed
the
sr_addOuterTryCatchInUpdateVpdKeyword
branch
from
74b9822 to
8e0beac
Compare
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.
This commit adds an outer try catch block in Parser::updateVpdKeyword API. Currently, this API has multiple return statements for different error scenarios, which reduces the API body's readability and modularity.
In order to streamline and handle all errors in a single location, an outer try catch has been added in the API body.
Test:
'''
Trigger this API by calling WriteKeyword API from D-Bus:
root@testhostname2:~# busctl call com.ibm.VPD.Manager /com/ibm/VPD/Manager com.ibm.VPD.Manager ReadKeyword sv "/sys/bus/i2c/drivers/at24/8-0050/eeprom" (ss) "UTIL" "D0" v ay 1 1
root@testhostname2:~# busctl call com.ibm.VPD.Manager /com/ibm/VPD/Manager com.ibm.VPD.Manager WriteKeyword sv "/sys/bus/i2c/drivers/at24/8-0050/eeprom" (ssay) "UTIL" "D0" 1 0 i 1
root@testhostname2:~# busctl call com.ibm.VPD.Manager /com/ibm/VPD/Manager com.ibm.VPD.Manager ReadKeyword sv "/sys/bus/i2c/drivers/at24/8-0050/eeprom" (ss) "UTIL" "D0" v ay 1 0
'''
Change-Id: I66a6c95ca0daaebb0a6f5c80e28ad4e9706d776f