Re: [Starlingx-discuss] Improve FM's Python API return data
Hi Eric, Thanks for the detail comments. In my plan, the return value for get_fault API will be like below: (False, None): there is error in the API execution. (True, None): API execution correctly, and there is no alarm message. (True, Alarm): API execution correctly, and there is alarm message. I think it should fix the issue you have. Is it right? If you agree on this new API return value change, I will implement it. Thanks. Best Regards Shuicheng From: MacDonald, Eric [mailto:Eric.MacDonald@windriver.com] Sent: Tuesday, February 19, 2019 10:35 PM To: Lin, Shuicheng <shuicheng.lin@intel.com>; Liu, Tao <Tao.Liu@windriver.com> Cc: Xie, Cindy <cindy.xie@intel.com> Subject: RE: Improve FM's Python API return data I took a look and added comments. I don't think this update fixes the issue I have. From: Lin, Shuicheng [mailto:shuicheng.lin@intel.com] Sent: Monday, February 18, 2019 10:09 PM To: MacDonald, Eric; Liu, Tao Cc: Xie, Cindy Subject: RE: Improve FM's Python API return data Importance: High Resend the mail. Best Regards Shuicheng From: Lin, Shuicheng Sent: Tuesday, February 19, 2019 9:56 AM To: Liu, Tao (Wind River) <tao.liu@windriver.com<mailto:tao.liu@windriver.com>>; 'eric.macdonald@windriver.com' <eric.macdonald@windriver.com<mailto:eric.macdonald@windriver.com>> Cc: Xie, Cindy <cindy.xie@intel.com<mailto:cindy.xie@intel.com>> Subject: Improve FM's Python API return data Hi Tao/Eric, For story 2004859<https://storyboard.openstack.org/#!/story/2004859>, I plan to modify the get_fault api return value as a tuple. (True/False, Alarm) Alarm is valid only when the 1st item is True. Sample code is uploaded for you early review: Stx-fault: https://review.openstack.org/637655 Stx-integ: https://review.openstack.org/637656 Could you help review it and share your thought? If you agree on it, I will implement the code and test it. For test, I will try to kill fmManager/modify its listening port to simulate the failure case. For the api call outside of stx-fault, I may try to call it manually to verify. Please share me if there is better verify method. Thanks. Best Regards Shuicheng
Hi all, Based on the comments in patch [0], I will add two new API, get_fault_ex and get_faults_by_id_ex. I will not add a new class, since all other APIs are kept without change. For get_fault_ex and get_faults_by_id_ex: the return value will be alarm list (maybe None). Exception will be thrown if there is error during API execution. [0]: https://review.openstack.org/637655 Best Regards Shuicheng From: Lin, Shuicheng [mailto:shuicheng.lin@intel.com] Sent: Wednesday, February 20, 2019 10:41 AM To: MacDonald, Eric <Eric.MacDonald@windriver.com>; Liu, Tao <Tao.Liu@windriver.com>; Al.Bailey@windriver.com Cc: Xie, Cindy <cindy.xie@intel.com>; starlingx-discuss@lists.starlingx.io Subject: Re: [Starlingx-discuss] Improve FM's Python API return data Hi Eric, Thanks for the detail comments. In my plan, the return value for get_fault API will be like below: (False, None): there is error in the API execution. (True, None): API execution correctly, and there is no alarm message. (True, Alarm): API execution correctly, and there is alarm message. I think it should fix the issue you have. Is it right? If you agree on this new API return value change, I will implement it. Thanks. Best Regards Shuicheng From: MacDonald, Eric [mailto:Eric.MacDonald@windriver.com] Sent: Tuesday, February 19, 2019 10:35 PM To: Lin, Shuicheng <shuicheng.lin@intel.com<mailto:shuicheng.lin@intel.com>>; Liu, Tao <Tao.Liu@windriver.com<mailto:Tao.Liu@windriver.com>> Cc: Xie, Cindy <cindy.xie@intel.com<mailto:cindy.xie@intel.com>> Subject: RE: Improve FM's Python API return data I took a look and added comments. I don't think this update fixes the issue I have. From: Lin, Shuicheng [mailto:shuicheng.lin@intel.com] Sent: Monday, February 18, 2019 10:09 PM To: MacDonald, Eric; Liu, Tao Cc: Xie, Cindy Subject: RE: Improve FM's Python API return data Importance: High Resend the mail. Best Regards Shuicheng From: Lin, Shuicheng Sent: Tuesday, February 19, 2019 9:56 AM To: Liu, Tao (Wind River) <tao.liu@windriver.com<mailto:tao.liu@windriver.com>>; 'eric.macdonald@windriver.com' <eric.macdonald@windriver.com<mailto:eric.macdonald@windriver.com>> Cc: Xie, Cindy <cindy.xie@intel.com<mailto:cindy.xie@intel.com>> Subject: Improve FM's Python API return data Hi Tao/Eric, For story 2004859<https://storyboard.openstack.org/#!/story/2004859>, I plan to modify the get_fault api return value as a tuple. (True/False, Alarm) Alarm is valid only when the 1st item is True. Sample code is uploaded for you early review: Stx-fault: https://review.openstack.org/637655 Stx-integ: https://review.openstack.org/637656 Could you help review it and share your thought? If you agree on it, I will implement the code and test it. For test, I will try to kill fmManager/modify its listening port to simulate the failure case. For the api call outside of stx-fault, I may try to call it manually to verify. Please share me if there is better verify method. Thanks. Best Regards Shuicheng
I disagree with this approach. Instead of just updating two of the methods, you should be updating all the methods in the FaultAPIs class that could fail in the same way and have them raise an exception. You should create a new FaultAPIs_v2 class for the updated methods to avoid impacting existing users. When a user of this API wants to use the new exceptions, they will start using the new class. Bart From: Lin, Shuicheng [mailto:shuicheng.lin@intel.com] Sent: February 25, 2019 2:58 AM To: MacDonald, Eric; Liu, Tao; Bailey, Henry Albert (Al); Wensley, Barton Cc: Xie, Cindy; starlingx-discuss@lists.starlingx.io Subject: RE: Improve FM's Python API return data Hi all, Based on the comments in patch [0], I will add two new API, get_fault_ex and get_faults_by_id_ex. I will not add a new class, since all other APIs are kept without change. For get_fault_ex and get_faults_by_id_ex: the return value will be alarm list (maybe None). Exception will be thrown if there is error during API execution. [0]: https://review.openstack.org/637655 Best Regards Shuicheng From: Lin, Shuicheng [mailto:shuicheng.lin@intel.com] Sent: Wednesday, February 20, 2019 10:41 AM To: MacDonald, Eric <Eric.MacDonald@windriver.com>; Liu, Tao <Tao.Liu@windriver.com>; Al.Bailey@windriver.com Cc: Xie, Cindy <cindy.xie@intel.com>; starlingx-discuss@lists.starlingx.io Subject: Re: [Starlingx-discuss] Improve FM's Python API return data Hi Eric, Thanks for the detail comments. In my plan, the return value for get_fault API will be like below: (False, None): there is error in the API execution. (True, None): API execution correctly, and there is no alarm message. (True, Alarm): API execution correctly, and there is alarm message. I think it should fix the issue you have. Is it right? If you agree on this new API return value change, I will implement it. Thanks. Best Regards Shuicheng From: MacDonald, Eric [mailto:Eric.MacDonald@windriver.com] Sent: Tuesday, February 19, 2019 10:35 PM To: Lin, Shuicheng <shuicheng.lin@intel.com<mailto:shuicheng.lin@intel.com>>; Liu, Tao <Tao.Liu@windriver.com<mailto:Tao.Liu@windriver.com>> Cc: Xie, Cindy <cindy.xie@intel.com<mailto:cindy.xie@intel.com>> Subject: RE: Improve FM's Python API return data I took a look and added comments. I don't think this update fixes the issue I have. From: Lin, Shuicheng [mailto:shuicheng.lin@intel.com] Sent: Monday, February 18, 2019 10:09 PM To: MacDonald, Eric; Liu, Tao Cc: Xie, Cindy Subject: RE: Improve FM's Python API return data Importance: High Resend the mail. Best Regards Shuicheng From: Lin, Shuicheng Sent: Tuesday, February 19, 2019 9:56 AM To: Liu, Tao (Wind River) <tao.liu@windriver.com<mailto:tao.liu@windriver.com>>; 'eric.macdonald@windriver.com' <eric.macdonald@windriver.com<mailto:eric.macdonald@windriver.com>> Cc: Xie, Cindy <cindy.xie@intel.com<mailto:cindy.xie@intel.com>> Subject: Improve FM's Python API return data Hi Tao/Eric, For story 2004859<https://storyboard.openstack.org/#!/story/2004859>, I plan to modify the get_fault api return value as a tuple. (True/False, Alarm) Alarm is valid only when the 1st item is True. Sample code is uploaded for you early review: Stx-fault: https://review.openstack.org/637655 Stx-integ: https://review.openstack.org/637656 Could you help review it and share your thought? If you agree on it, I will implement the code and test it. For test, I will try to kill fmManager/modify its listening port to simulate the failure case. For the api call outside of stx-fault, I may try to call it manually to verify. Please share me if there is better verify method. Thanks. Best Regards Shuicheng
I see your point Bart. This Jira is an Enhancement Request and fulfilling that should not be done so half way. All the APIs in V2 should be changed to defer exception handling to the caller as apparently that is the pythonic way. From: Wensley, Barton Sent: Monday, February 25, 2019 8:10 AM To: Lin, Shuicheng; MacDonald, Eric; Liu, Tao; Bailey, Henry Albert (Al) Cc: Xie, Cindy; starlingx-discuss@lists.starlingx.io Subject: RE: Improve FM's Python API return data Importance: High I disagree with this approach. Instead of just updating two of the methods, you should be updating all the methods in the FaultAPIs class that could fail in the same way and have them raise an exception. You should create a new FaultAPIs_v2 class for the updated methods to avoid impacting existing users. When a user of this API wants to use the new exceptions, they will start using the new class. Bart From: Lin, Shuicheng [mailto:shuicheng.lin@intel.com] Sent: February 25, 2019 2:58 AM To: MacDonald, Eric; Liu, Tao; Bailey, Henry Albert (Al); Wensley, Barton Cc: Xie, Cindy; starlingx-discuss@lists.starlingx.io Subject: RE: Improve FM's Python API return data Hi all, Based on the comments in patch [0], I will add two new API, get_fault_ex and get_faults_by_id_ex. I will not add a new class, since all other APIs are kept without change. For get_fault_ex and get_faults_by_id_ex: the return value will be alarm list (maybe None). Exception will be thrown if there is error during API execution. [0]: https://review.openstack.org/637655 Best Regards Shuicheng From: Lin, Shuicheng [mailto:shuicheng.lin@intel.com] Sent: Wednesday, February 20, 2019 10:41 AM To: MacDonald, Eric <Eric.MacDonald@windriver.com>; Liu, Tao <Tao.Liu@windriver.com>; Al.Bailey@windriver.com Cc: Xie, Cindy <cindy.xie@intel.com>; starlingx-discuss@lists.starlingx.io Subject: Re: [Starlingx-discuss] Improve FM's Python API return data Hi Eric, Thanks for the detail comments. In my plan, the return value for get_fault API will be like below: (False, None): there is error in the API execution. (True, None): API execution correctly, and there is no alarm message. (True, Alarm): API execution correctly, and there is alarm message. I think it should fix the issue you have. Is it right? If you agree on this new API return value change, I will implement it. Thanks. Best Regards Shuicheng From: MacDonald, Eric [mailto:Eric.MacDonald@windriver.com] Sent: Tuesday, February 19, 2019 10:35 PM To: Lin, Shuicheng <shuicheng.lin@intel.com<mailto:shuicheng.lin@intel.com>>; Liu, Tao <Tao.Liu@windriver.com<mailto:Tao.Liu@windriver.com>> Cc: Xie, Cindy <cindy.xie@intel.com<mailto:cindy.xie@intel.com>> Subject: RE: Improve FM's Python API return data I took a look and added comments. I don't think this update fixes the issue I have. From: Lin, Shuicheng [mailto:shuicheng.lin@intel.com] Sent: Monday, February 18, 2019 10:09 PM To: MacDonald, Eric; Liu, Tao Cc: Xie, Cindy Subject: RE: Improve FM's Python API return data Importance: High Resend the mail. Best Regards Shuicheng From: Lin, Shuicheng Sent: Tuesday, February 19, 2019 9:56 AM To: Liu, Tao (Wind River) <tao.liu@windriver.com<mailto:tao.liu@windriver.com>>; 'eric.macdonald@windriver.com' <eric.macdonald@windriver.com<mailto:eric.macdonald@windriver.com>> Cc: Xie, Cindy <cindy.xie@intel.com<mailto:cindy.xie@intel.com>> Subject: Improve FM's Python API return data Hi Tao/Eric, For story 2004859<https://storyboard.openstack.org/#!/story/2004859>, I plan to modify the get_fault api return value as a tuple. (True/False, Alarm) Alarm is valid only when the 1st item is True. Sample code is uploaded for you early review: Stx-fault: https://review.openstack.org/637655 Stx-integ: https://review.openstack.org/637656 Could you help review it and share your thought? If you agree on it, I will implement the code and test it. For test, I will try to kill fmManager/modify its listening port to simulate the failure case. For the api call outside of stx-fault, I may try to call it manually to verify. Please share me if there is better verify method. Thanks. Best Regards Shuicheng
Hi Eric/Tao/Al, Please confirm you agree a new set fault API (FaultAPIs_V2) or not. The attached excel is a simple comparison between original class FaultAPIs and FaultAPIs_V2. For myself, I still prefer to add 2 new API in original class, so other API call could be kept without change. We could have more discuss about the FaultAPIs_V2 design if you all agree to choose it. Best Regards Shuicheng From: MacDonald, Eric [mailto:Eric.MacDonald@windriver.com] Sent: Monday, February 25, 2019 10:23 PM To: Wensley, Barton <Barton.Wensley@windriver.com>; Lin, Shuicheng <shuicheng.lin@intel.com>; Liu, Tao <Tao.Liu@windriver.com>; Bailey, Henry Albert (Al) <Al.Bailey@windriver.com> Cc: Xie, Cindy <cindy.xie@intel.com>; starlingx-discuss@lists.starlingx.io Subject: RE: Improve FM's Python API return data I see your point Bart. This Jira is an Enhancement Request and fulfilling that should not be done so half way. All the APIs in V2 should be changed to defer exception handling to the caller as apparently that is the pythonic way. From: Wensley, Barton Sent: Monday, February 25, 2019 8:10 AM To: Lin, Shuicheng; MacDonald, Eric; Liu, Tao; Bailey, Henry Albert (Al) Cc: Xie, Cindy; starlingx-discuss@lists.starlingx.io<mailto:starlingx-discuss@lists.starlingx.io> Subject: RE: Improve FM's Python API return data Importance: High I disagree with this approach. Instead of just updating two of the methods, you should be updating all the methods in the FaultAPIs class that could fail in the same way and have them raise an exception. You should create a new FaultAPIs_v2 class for the updated methods to avoid impacting existing users. When a user of this API wants to use the new exceptions, they will start using the new class. Bart From: Lin, Shuicheng [mailto:shuicheng.lin@intel.com] Sent: February 25, 2019 2:58 AM To: MacDonald, Eric; Liu, Tao; Bailey, Henry Albert (Al); Wensley, Barton Cc: Xie, Cindy; starlingx-discuss@lists.starlingx.io<mailto:starlingx-discuss@lists.starlingx.io> Subject: RE: Improve FM's Python API return data Hi all, Based on the comments in patch [0], I will add two new API, get_fault_ex and get_faults_by_id_ex. I will not add a new class, since all other APIs are kept without change. For get_fault_ex and get_faults_by_id_ex: the return value will be alarm list (maybe None). Exception will be thrown if there is error during API execution. [0]: https://review.openstack.org/637655 Best Regards Shuicheng From: Lin, Shuicheng [mailto:shuicheng.lin@intel.com] Sent: Wednesday, February 20, 2019 10:41 AM To: MacDonald, Eric <Eric.MacDonald@windriver.com<mailto:Eric.MacDonald@windriver.com>>; Liu, Tao <Tao.Liu@windriver.com<mailto:Tao.Liu@windriver.com>>; Al.Bailey@windriver.com<mailto:Al.Bailey@windriver.com> Cc: Xie, Cindy <cindy.xie@intel.com<mailto:cindy.xie@intel.com>>; starlingx-discuss@lists.starlingx.io<mailto:starlingx-discuss@lists.starlingx.io> Subject: Re: [Starlingx-discuss] Improve FM's Python API return data Hi Eric, Thanks for the detail comments. In my plan, the return value for get_fault API will be like below: (False, None): there is error in the API execution. (True, None): API execution correctly, and there is no alarm message. (True, Alarm): API execution correctly, and there is alarm message. I think it should fix the issue you have. Is it right? If you agree on this new API return value change, I will implement it. Thanks. Best Regards Shuicheng From: MacDonald, Eric [mailto:Eric.MacDonald@windriver.com] Sent: Tuesday, February 19, 2019 10:35 PM To: Lin, Shuicheng <shuicheng.lin@intel.com<mailto:shuicheng.lin@intel.com>>; Liu, Tao <Tao.Liu@windriver.com<mailto:Tao.Liu@windriver.com>> Cc: Xie, Cindy <cindy.xie@intel.com<mailto:cindy.xie@intel.com>> Subject: RE: Improve FM's Python API return data I took a look and added comments. I don't think this update fixes the issue I have. From: Lin, Shuicheng [mailto:shuicheng.lin@intel.com] Sent: Monday, February 18, 2019 10:09 PM To: MacDonald, Eric; Liu, Tao Cc: Xie, Cindy Subject: RE: Improve FM's Python API return data Importance: High Resend the mail. Best Regards Shuicheng From: Lin, Shuicheng Sent: Tuesday, February 19, 2019 9:56 AM To: Liu, Tao (Wind River) <tao.liu@windriver.com<mailto:tao.liu@windriver.com>>; 'eric.macdonald@windriver.com' <eric.macdonald@windriver.com<mailto:eric.macdonald@windriver.com>> Cc: Xie, Cindy <cindy.xie@intel.com<mailto:cindy.xie@intel.com>> Subject: Improve FM's Python API return data Hi Tao/Eric, For story 2004859<https://storyboard.openstack.org/#!/story/2004859>, I plan to modify the get_fault api return value as a tuple. (True/False, Alarm) Alarm is valid only when the 1st item is True. Sample code is uploaded for you early review: Stx-fault: https://review.openstack.org/637655 Stx-integ: https://review.openstack.org/637656 Could you help review it and share your thought? If you agree on it, I will implement the code and test it. For test, I will try to kill fmManager/modify its listening port to simulate the failure case. For the api call outside of stx-fault, I may try to call it manually to verify. Please share me if there is better verify method. Thanks. Best Regards Shuicheng
Shuicheng, As per your spreadsheet, there are actually three APIs that currently return ambiguous results (get_fault, get_faults and get_faults_by_id). My view is that you should be updating all six APIs to be consistent and raise an exception if the operation fails (e.g. because the API can't connect to fmmanager). You can avoid impacting the existing APIs by having both a FaultAPIs class and a FaultAPIsV2 class. To avoid code duplication, you would create a FaultAPIsBase class to hold all the common code and subclass from it. Bart From: Lin, Shuicheng [mailto:shuicheng.lin@intel.com] Sent: February 25, 2019 8:59 PM To: MacDonald, Eric; Wensley, Barton; Liu, Tao; Bailey, Henry Albert (Al) Cc: Xie, Cindy; starlingx-discuss@lists.starlingx.io Subject: RE: Improve FM's Python API return data Hi Eric/Tao/Al, Please confirm you agree a new set fault API (FaultAPIs_V2) or not. The attached excel is a simple comparison between original class FaultAPIs and FaultAPIs_V2. For myself, I still prefer to add 2 new API in original class, so other API call could be kept without change. We could have more discuss about the FaultAPIs_V2 design if you all agree to choose it. Best Regards Shuicheng From: MacDonald, Eric [mailto:Eric.MacDonald@windriver.com] Sent: Monday, February 25, 2019 10:23 PM To: Wensley, Barton <Barton.Wensley@windriver.com>; Lin, Shuicheng <shuicheng.lin@intel.com>; Liu, Tao <Tao.Liu@windriver.com>; Bailey, Henry Albert (Al) <Al.Bailey@windriver.com> Cc: Xie, Cindy <cindy.xie@intel.com>; starlingx-discuss@lists.starlingx.io Subject: RE: Improve FM's Python API return data I see your point Bart. This Jira is an Enhancement Request and fulfilling that should not be done so half way. All the APIs in V2 should be changed to defer exception handling to the caller as apparently that is the pythonic way. From: Wensley, Barton Sent: Monday, February 25, 2019 8:10 AM To: Lin, Shuicheng; MacDonald, Eric; Liu, Tao; Bailey, Henry Albert (Al) Cc: Xie, Cindy; starlingx-discuss@lists.starlingx.io<mailto:starlingx-discuss@lists.starlingx.io> Subject: RE: Improve FM's Python API return data Importance: High I disagree with this approach. Instead of just updating two of the methods, you should be updating all the methods in the FaultAPIs class that could fail in the same way and have them raise an exception. You should create a new FaultAPIs_v2 class for the updated methods to avoid impacting existing users. When a user of this API wants to use the new exceptions, they will start using the new class. Bart From: Lin, Shuicheng [mailto:shuicheng.lin@intel.com] Sent: February 25, 2019 2:58 AM To: MacDonald, Eric; Liu, Tao; Bailey, Henry Albert (Al); Wensley, Barton Cc: Xie, Cindy; starlingx-discuss@lists.starlingx.io<mailto:starlingx-discuss@lists.starlingx.io> Subject: RE: Improve FM's Python API return data Hi all, Based on the comments in patch [0], I will add two new API, get_fault_ex and get_faults_by_id_ex. I will not add a new class, since all other APIs are kept without change. For get_fault_ex and get_faults_by_id_ex: the return value will be alarm list (maybe None). Exception will be thrown if there is error during API execution. [0]: https://review.openstack.org/637655 Best Regards Shuicheng From: Lin, Shuicheng [mailto:shuicheng.lin@intel.com] Sent: Wednesday, February 20, 2019 10:41 AM To: MacDonald, Eric <Eric.MacDonald@windriver.com<mailto:Eric.MacDonald@windriver.com>>; Liu, Tao <Tao.Liu@windriver.com<mailto:Tao.Liu@windriver.com>>; Al.Bailey@windriver.com<mailto:Al.Bailey@windriver.com> Cc: Xie, Cindy <cindy.xie@intel.com<mailto:cindy.xie@intel.com>>; starlingx-discuss@lists.starlingx.io<mailto:starlingx-discuss@lists.starlingx.io> Subject: Re: [Starlingx-discuss] Improve FM's Python API return data Hi Eric, Thanks for the detail comments. In my plan, the return value for get_fault API will be like below: (False, None): there is error in the API execution. (True, None): API execution correctly, and there is no alarm message. (True, Alarm): API execution correctly, and there is alarm message. I think it should fix the issue you have. Is it right? If you agree on this new API return value change, I will implement it. Thanks. Best Regards Shuicheng From: MacDonald, Eric [mailto:Eric.MacDonald@windriver.com] Sent: Tuesday, February 19, 2019 10:35 PM To: Lin, Shuicheng <shuicheng.lin@intel.com<mailto:shuicheng.lin@intel.com>>; Liu, Tao <Tao.Liu@windriver.com<mailto:Tao.Liu@windriver.com>> Cc: Xie, Cindy <cindy.xie@intel.com<mailto:cindy.xie@intel.com>> Subject: RE: Improve FM's Python API return data I took a look and added comments. I don't think this update fixes the issue I have. From: Lin, Shuicheng [mailto:shuicheng.lin@intel.com] Sent: Monday, February 18, 2019 10:09 PM To: MacDonald, Eric; Liu, Tao Cc: Xie, Cindy Subject: RE: Improve FM's Python API return data Importance: High Resend the mail. Best Regards Shuicheng From: Lin, Shuicheng Sent: Tuesday, February 19, 2019 9:56 AM To: Liu, Tao (Wind River) <tao.liu@windriver.com<mailto:tao.liu@windriver.com>>; 'eric.macdonald@windriver.com' <eric.macdonald@windriver.com<mailto:eric.macdonald@windriver.com>> Cc: Xie, Cindy <cindy.xie@intel.com<mailto:cindy.xie@intel.com>> Subject: Improve FM's Python API return data Hi Tao/Eric, For story 2004859<https://storyboard.openstack.org/#!/story/2004859>, I plan to modify the get_fault api return value as a tuple. (True/False, Alarm) Alarm is valid only when the 1st item is True. Sample code is uploaded for you early review: Stx-fault: https://review.openstack.org/637655 Stx-integ: https://review.openstack.org/637656 Could you help review it and share your thought? If you agree on it, I will implement the code and test it. For test, I will try to kill fmManager/modify its listening port to simulate the failure case. For the api call outside of stx-fault, I may try to call it manually to verify. Please share me if there is better verify method. Thanks. Best Regards Shuicheng
I agree with Bart about this. V1 returns what it does now to minimize impact on existing API consumers, V2 returns your new structure so those consumers can migrate to that new result format if they choose. Al From: Wensley, Barton Sent: Tuesday, February 26, 2019 9:11 AM To: Lin, Shuicheng; MacDonald, Eric; Liu, Tao; Bailey, Henry Albert (Al) Cc: Xie, Cindy; starlingx-discuss@lists.starlingx.io Subject: RE: Improve FM's Python API return data Shuicheng, As per your spreadsheet, there are actually three APIs that currently return ambiguous results (get_fault, get_faults and get_faults_by_id). My view is that you should be updating all six APIs to be consistent and raise an exception if the operation fails (e.g. because the API can't connect to fmmanager). You can avoid impacting the existing APIs by having both a FaultAPIs class and a FaultAPIsV2 class. To avoid code duplication, you would create a FaultAPIsBase class to hold all the common code and subclass from it. Bart From: Lin, Shuicheng [mailto:shuicheng.lin@intel.com] Sent: February 25, 2019 8:59 PM To: MacDonald, Eric; Wensley, Barton; Liu, Tao; Bailey, Henry Albert (Al) Cc: Xie, Cindy; starlingx-discuss@lists.starlingx.io Subject: RE: Improve FM's Python API return data Hi Eric/Tao/Al, Please confirm you agree a new set fault API (FaultAPIs_V2) or not. The attached excel is a simple comparison between original class FaultAPIs and FaultAPIs_V2. For myself, I still prefer to add 2 new API in original class, so other API call could be kept without change. We could have more discuss about the FaultAPIs_V2 design if you all agree to choose it. Best Regards Shuicheng From: MacDonald, Eric [mailto:Eric.MacDonald@windriver.com] Sent: Monday, February 25, 2019 10:23 PM To: Wensley, Barton <Barton.Wensley@windriver.com>; Lin, Shuicheng <shuicheng.lin@intel.com>; Liu, Tao <Tao.Liu@windriver.com>; Bailey, Henry Albert (Al) <Al.Bailey@windriver.com> Cc: Xie, Cindy <cindy.xie@intel.com>; starlingx-discuss@lists.starlingx.io Subject: RE: Improve FM's Python API return data I see your point Bart. This Jira is an Enhancement Request and fulfilling that should not be done so half way. All the APIs in V2 should be changed to defer exception handling to the caller as apparently that is the pythonic way. From: Wensley, Barton Sent: Monday, February 25, 2019 8:10 AM To: Lin, Shuicheng; MacDonald, Eric; Liu, Tao; Bailey, Henry Albert (Al) Cc: Xie, Cindy; starlingx-discuss@lists.starlingx.io<mailto:starlingx-discuss@lists.starlingx.io> Subject: RE: Improve FM's Python API return data Importance: High I disagree with this approach. Instead of just updating two of the methods, you should be updating all the methods in the FaultAPIs class that could fail in the same way and have them raise an exception. You should create a new FaultAPIs_v2 class for the updated methods to avoid impacting existing users. When a user of this API wants to use the new exceptions, they will start using the new class. Bart From: Lin, Shuicheng [mailto:shuicheng.lin@intel.com] Sent: February 25, 2019 2:58 AM To: MacDonald, Eric; Liu, Tao; Bailey, Henry Albert (Al); Wensley, Barton Cc: Xie, Cindy; starlingx-discuss@lists.starlingx.io<mailto:starlingx-discuss@lists.starlingx.io> Subject: RE: Improve FM's Python API return data Hi all, Based on the comments in patch [0], I will add two new API, get_fault_ex and get_faults_by_id_ex. I will not add a new class, since all other APIs are kept without change. For get_fault_ex and get_faults_by_id_ex: the return value will be alarm list (maybe None). Exception will be thrown if there is error during API execution. [0]: https://review.openstack.org/637655 Best Regards Shuicheng From: Lin, Shuicheng [mailto:shuicheng.lin@intel.com] Sent: Wednesday, February 20, 2019 10:41 AM To: MacDonald, Eric <Eric.MacDonald@windriver.com<mailto:Eric.MacDonald@windriver.com>>; Liu, Tao <Tao.Liu@windriver.com<mailto:Tao.Liu@windriver.com>>; Al.Bailey@windriver.com<mailto:Al.Bailey@windriver.com> Cc: Xie, Cindy <cindy.xie@intel.com<mailto:cindy.xie@intel.com>>; starlingx-discuss@lists.starlingx.io<mailto:starlingx-discuss@lists.starlingx.io> Subject: Re: [Starlingx-discuss] Improve FM's Python API return data Hi Eric, Thanks for the detail comments. In my plan, the return value for get_fault API will be like below: (False, None): there is error in the API execution. (True, None): API execution correctly, and there is no alarm message. (True, Alarm): API execution correctly, and there is alarm message. I think it should fix the issue you have. Is it right? If you agree on this new API return value change, I will implement it. Thanks. Best Regards Shuicheng From: MacDonald, Eric [mailto:Eric.MacDonald@windriver.com] Sent: Tuesday, February 19, 2019 10:35 PM To: Lin, Shuicheng <shuicheng.lin@intel.com<mailto:shuicheng.lin@intel.com>>; Liu, Tao <Tao.Liu@windriver.com<mailto:Tao.Liu@windriver.com>> Cc: Xie, Cindy <cindy.xie@intel.com<mailto:cindy.xie@intel.com>> Subject: RE: Improve FM's Python API return data I took a look and added comments. I don't think this update fixes the issue I have. From: Lin, Shuicheng [mailto:shuicheng.lin@intel.com] Sent: Monday, February 18, 2019 10:09 PM To: MacDonald, Eric; Liu, Tao Cc: Xie, Cindy Subject: RE: Improve FM's Python API return data Importance: High Resend the mail. Best Regards Shuicheng From: Lin, Shuicheng Sent: Tuesday, February 19, 2019 9:56 AM To: Liu, Tao (Wind River) <tao.liu@windriver.com<mailto:tao.liu@windriver.com>>; 'eric.macdonald@windriver.com' <eric.macdonald@windriver.com<mailto:eric.macdonald@windriver.com>> Cc: Xie, Cindy <cindy.xie@intel.com<mailto:cindy.xie@intel.com>> Subject: Improve FM's Python API return data Hi Tao/Eric, For story 2004859<https://storyboard.openstack.org/#!/story/2004859>, I plan to modify the get_fault api return value as a tuple. (True/False, Alarm) Alarm is valid only when the 1st item is True. Sample code is uploaded for you early review: Stx-fault: https://review.openstack.org/637655 Stx-integ: https://review.openstack.org/637656 Could you help review it and share your thought? If you agree on it, I will implement the code and test it. For test, I will try to kill fmManager/modify its listening port to simulate the failure case. For the api call outside of stx-fault, I may try to call it manually to verify. Please share me if there is better verify method. Thanks. Best Regards Shuicheng
Agreed. From: Wensley, Barton Sent: Tuesday, February 26, 2019 9:11 AM To: Lin, Shuicheng; MacDonald, Eric; Liu, Tao; Bailey, Henry Albert (Al) Cc: Xie, Cindy; starlingx-discuss@lists.starlingx.io Subject: RE: Improve FM's Python API return data Importance: High Shuicheng, As per your spreadsheet, there are actually three APIs that currently return ambiguous results (get_fault, get_faults and get_faults_by_id). My view is that you should be updating all six APIs to be consistent and raise an exception if the operation fails (e.g. because the API can't connect to fmmanager). You can avoid impacting the existing APIs by having both a FaultAPIs class and a FaultAPIsV2 class. To avoid code duplication, you would create a FaultAPIsBase class to hold all the common code and subclass from it. Bart From: Lin, Shuicheng [mailto:shuicheng.lin@intel.com] Sent: February 25, 2019 8:59 PM To: MacDonald, Eric; Wensley, Barton; Liu, Tao; Bailey, Henry Albert (Al) Cc: Xie, Cindy; starlingx-discuss@lists.starlingx.io Subject: RE: Improve FM's Python API return data Hi Eric/Tao/Al, Please confirm you agree a new set fault API (FaultAPIs_V2) or not. The attached excel is a simple comparison between original class FaultAPIs and FaultAPIs_V2. For myself, I still prefer to add 2 new API in original class, so other API call could be kept without change. We could have more discuss about the FaultAPIs_V2 design if you all agree to choose it. Best Regards Shuicheng From: MacDonald, Eric [mailto:Eric.MacDonald@windriver.com] Sent: Monday, February 25, 2019 10:23 PM To: Wensley, Barton <Barton.Wensley@windriver.com>; Lin, Shuicheng <shuicheng.lin@intel.com>; Liu, Tao <Tao.Liu@windriver.com>; Bailey, Henry Albert (Al) <Al.Bailey@windriver.com> Cc: Xie, Cindy <cindy.xie@intel.com>; starlingx-discuss@lists.starlingx.io Subject: RE: Improve FM's Python API return data I see your point Bart. This Jira is an Enhancement Request and fulfilling that should not be done so half way. All the APIs in V2 should be changed to defer exception handling to the caller as apparently that is the pythonic way. From: Wensley, Barton Sent: Monday, February 25, 2019 8:10 AM To: Lin, Shuicheng; MacDonald, Eric; Liu, Tao; Bailey, Henry Albert (Al) Cc: Xie, Cindy; starlingx-discuss@lists.starlingx.io<mailto:starlingx-discuss@lists.starlingx.io> Subject: RE: Improve FM's Python API return data Importance: High I disagree with this approach. Instead of just updating two of the methods, you should be updating all the methods in the FaultAPIs class that could fail in the same way and have them raise an exception. You should create a new FaultAPIs_v2 class for the updated methods to avoid impacting existing users. When a user of this API wants to use the new exceptions, they will start using the new class. Bart From: Lin, Shuicheng [mailto:shuicheng.lin@intel.com] Sent: February 25, 2019 2:58 AM To: MacDonald, Eric; Liu, Tao; Bailey, Henry Albert (Al); Wensley, Barton Cc: Xie, Cindy; starlingx-discuss@lists.starlingx.io<mailto:starlingx-discuss@lists.starlingx.io> Subject: RE: Improve FM's Python API return data Hi all, Based on the comments in patch [0], I will add two new API, get_fault_ex and get_faults_by_id_ex. I will not add a new class, since all other APIs are kept without change. For get_fault_ex and get_faults_by_id_ex: the return value will be alarm list (maybe None). Exception will be thrown if there is error during API execution. [0]: https://review.openstack.org/637655 Best Regards Shuicheng From: Lin, Shuicheng [mailto:shuicheng.lin@intel.com] Sent: Wednesday, February 20, 2019 10:41 AM To: MacDonald, Eric <Eric.MacDonald@windriver.com<mailto:Eric.MacDonald@windriver.com>>; Liu, Tao <Tao.Liu@windriver.com<mailto:Tao.Liu@windriver.com>>; Al.Bailey@windriver.com<mailto:Al.Bailey@windriver.com> Cc: Xie, Cindy <cindy.xie@intel.com<mailto:cindy.xie@intel.com>>; starlingx-discuss@lists.starlingx.io<mailto:starlingx-discuss@lists.starlingx.io> Subject: Re: [Starlingx-discuss] Improve FM's Python API return data Hi Eric, Thanks for the detail comments. In my plan, the return value for get_fault API will be like below: (False, None): there is error in the API execution. (True, None): API execution correctly, and there is no alarm message. (True, Alarm): API execution correctly, and there is alarm message. I think it should fix the issue you have. Is it right? If you agree on this new API return value change, I will implement it. Thanks. Best Regards Shuicheng From: MacDonald, Eric [mailto:Eric.MacDonald@windriver.com] Sent: Tuesday, February 19, 2019 10:35 PM To: Lin, Shuicheng <shuicheng.lin@intel.com<mailto:shuicheng.lin@intel.com>>; Liu, Tao <Tao.Liu@windriver.com<mailto:Tao.Liu@windriver.com>> Cc: Xie, Cindy <cindy.xie@intel.com<mailto:cindy.xie@intel.com>> Subject: RE: Improve FM's Python API return data I took a look and added comments. I don't think this update fixes the issue I have. From: Lin, Shuicheng [mailto:shuicheng.lin@intel.com] Sent: Monday, February 18, 2019 10:09 PM To: MacDonald, Eric; Liu, Tao Cc: Xie, Cindy Subject: RE: Improve FM's Python API return data Importance: High Resend the mail. Best Regards Shuicheng From: Lin, Shuicheng Sent: Tuesday, February 19, 2019 9:56 AM To: Liu, Tao (Wind River) <tao.liu@windriver.com<mailto:tao.liu@windriver.com>>; 'eric.macdonald@windriver.com' <eric.macdonald@windriver.com<mailto:eric.macdonald@windriver.com>> Cc: Xie, Cindy <cindy.xie@intel.com<mailto:cindy.xie@intel.com>> Subject: Improve FM's Python API return data Hi Tao/Eric, For story 2004859<https://storyboard.openstack.org/#!/story/2004859>, I plan to modify the get_fault api return value as a tuple. (True/False, Alarm) Alarm is valid only when the 1st item is True. Sample code is uploaded for you early review: Stx-fault: https://review.openstack.org/637655 Stx-integ: https://review.openstack.org/637656 Could you help review it and share your thought? If you agree on it, I will implement the code and test it. For test, I will try to kill fmManager/modify its listening port to simulate the failure case. For the api call outside of stx-fault, I may try to call it manually to verify. Please share me if there is better verify method. Thanks. Best Regards Shuicheng
Hi Shuicheng, I agree with Bart's recommendation. The failure condition occurs when the FM manager has not been started by SM during the booting or swact, and raise an exception for this condition is a right approach. Having a new class for the new behavior is a good way to migrate the API changes. Regards, Tao From: Wensley, Barton Sent: Tuesday, February 26, 2019 9:11 AM To: Lin, Shuicheng; MacDonald, Eric; Liu, Tao; Bailey, Henry Albert (Al) Cc: Xie, Cindy; starlingx-discuss@lists.starlingx.io Subject: RE: Improve FM's Python API return data Shuicheng, As per your spreadsheet, there are actually three APIs that currently return ambiguous results (get_fault, get_faults and get_faults_by_id). My view is that you should be updating all six APIs to be consistent and raise an exception if the operation fails (e.g. because the API can't connect to fmmanager). You can avoid impacting the existing APIs by having both a FaultAPIs class and a FaultAPIsV2 class. To avoid code duplication, you would create a FaultAPIsBase class to hold all the common code and subclass from it. Bart From: Lin, Shuicheng [mailto:shuicheng.lin@intel.com] Sent: February 25, 2019 8:59 PM To: MacDonald, Eric; Wensley, Barton; Liu, Tao; Bailey, Henry Albert (Al) Cc: Xie, Cindy; starlingx-discuss@lists.starlingx.io Subject: RE: Improve FM's Python API return data Hi Eric/Tao/Al, Please confirm you agree a new set fault API (FaultAPIs_V2) or not. The attached excel is a simple comparison between original class FaultAPIs and FaultAPIs_V2. For myself, I still prefer to add 2 new API in original class, so other API call could be kept without change. We could have more discuss about the FaultAPIs_V2 design if you all agree to choose it. Best Regards Shuicheng From: MacDonald, Eric [mailto:Eric.MacDonald@windriver.com] Sent: Monday, February 25, 2019 10:23 PM To: Wensley, Barton <Barton.Wensley@windriver.com>; Lin, Shuicheng <shuicheng.lin@intel.com>; Liu, Tao <Tao.Liu@windriver.com>; Bailey, Henry Albert (Al) <Al.Bailey@windriver.com> Cc: Xie, Cindy <cindy.xie@intel.com>; starlingx-discuss@lists.starlingx.io Subject: RE: Improve FM's Python API return data I see your point Bart. This Jira is an Enhancement Request and fulfilling that should not be done so half way. All the APIs in V2 should be changed to defer exception handling to the caller as apparently that is the pythonic way. From: Wensley, Barton Sent: Monday, February 25, 2019 8:10 AM To: Lin, Shuicheng; MacDonald, Eric; Liu, Tao; Bailey, Henry Albert (Al) Cc: Xie, Cindy; starlingx-discuss@lists.starlingx.io<mailto:starlingx-discuss@lists.starlingx.io> Subject: RE: Improve FM's Python API return data Importance: High I disagree with this approach. Instead of just updating two of the methods, you should be updating all the methods in the FaultAPIs class that could fail in the same way and have them raise an exception. You should create a new FaultAPIs_v2 class for the updated methods to avoid impacting existing users. When a user of this API wants to use the new exceptions, they will start using the new class. Bart From: Lin, Shuicheng [mailto:shuicheng.lin@intel.com] Sent: February 25, 2019 2:58 AM To: MacDonald, Eric; Liu, Tao; Bailey, Henry Albert (Al); Wensley, Barton Cc: Xie, Cindy; starlingx-discuss@lists.starlingx.io<mailto:starlingx-discuss@lists.starlingx.io> Subject: RE: Improve FM's Python API return data Hi all, Based on the comments in patch [0], I will add two new API, get_fault_ex and get_faults_by_id_ex. I will not add a new class, since all other APIs are kept without change. For get_fault_ex and get_faults_by_id_ex: the return value will be alarm list (maybe None). Exception will be thrown if there is error during API execution. [0]: https://review.openstack.org/637655 Best Regards Shuicheng From: Lin, Shuicheng [mailto:shuicheng.lin@intel.com] Sent: Wednesday, February 20, 2019 10:41 AM To: MacDonald, Eric <Eric.MacDonald@windriver.com<mailto:Eric.MacDonald@windriver.com>>; Liu, Tao <Tao.Liu@windriver.com<mailto:Tao.Liu@windriver.com>>; Al.Bailey@windriver.com<mailto:Al.Bailey@windriver.com> Cc: Xie, Cindy <cindy.xie@intel.com<mailto:cindy.xie@intel.com>>; starlingx-discuss@lists.starlingx.io<mailto:starlingx-discuss@lists.starlingx.io> Subject: Re: [Starlingx-discuss] Improve FM's Python API return data Hi Eric, Thanks for the detail comments. In my plan, the return value for get_fault API will be like below: (False, None): there is error in the API execution. (True, None): API execution correctly, and there is no alarm message. (True, Alarm): API execution correctly, and there is alarm message. I think it should fix the issue you have. Is it right? If you agree on this new API return value change, I will implement it. Thanks. Best Regards Shuicheng From: MacDonald, Eric [mailto:Eric.MacDonald@windriver.com] Sent: Tuesday, February 19, 2019 10:35 PM To: Lin, Shuicheng <shuicheng.lin@intel.com<mailto:shuicheng.lin@intel.com>>; Liu, Tao <Tao.Liu@windriver.com<mailto:Tao.Liu@windriver.com>> Cc: Xie, Cindy <cindy.xie@intel.com<mailto:cindy.xie@intel.com>> Subject: RE: Improve FM's Python API return data I took a look and added comments. I don't think this update fixes the issue I have. From: Lin, Shuicheng [mailto:shuicheng.lin@intel.com] Sent: Monday, February 18, 2019 10:09 PM To: MacDonald, Eric; Liu, Tao Cc: Xie, Cindy Subject: RE: Improve FM's Python API return data Importance: High Resend the mail. Best Regards Shuicheng From: Lin, Shuicheng Sent: Tuesday, February 19, 2019 9:56 AM To: Liu, Tao (Wind River) <tao.liu@windriver.com<mailto:tao.liu@windriver.com>>; 'eric.macdonald@windriver.com' <eric.macdonald@windriver.com<mailto:eric.macdonald@windriver.com>> Cc: Xie, Cindy <cindy.xie@intel.com<mailto:cindy.xie@intel.com>> Subject: Improve FM's Python API return data Hi Tao/Eric, For story 2004859<https://storyboard.openstack.org/#!/story/2004859>, I plan to modify the get_fault api return value as a tuple. (True/False, Alarm) Alarm is valid only when the 1st item is True. Sample code is uploaded for you early review: Stx-fault: https://review.openstack.org/637655 Stx-integ: https://review.openstack.org/637656 Could you help review it and share your thought? If you agree on it, I will implement the code and test it. For test, I will try to kill fmManager/modify its listening port to simulate the failure case. For the api call outside of stx-fault, I may try to call it manually to verify. Please share me if there is better verify method. Thanks. Best Regards Shuicheng
Hi all, Thanks for the quick confirmation. I will implement the code as Bart's suggestion. Best Regards Shuicheng From: Liu, Tao [mailto:Tao.Liu@windriver.com] Sent: Tuesday, February 26, 2019 10:31 PM To: Wensley, Barton <Barton.Wensley@windriver.com>; Lin, Shuicheng <shuicheng.lin@intel.com>; MacDonald, Eric <Eric.MacDonald@windriver.com>; Bailey, Henry Albert (Al) <Al.Bailey@windriver.com> Cc: Xie, Cindy <cindy.xie@intel.com>; starlingx-discuss@lists.starlingx.io Subject: RE: Improve FM's Python API return data Hi Shuicheng, I agree with Bart's recommendation. The failure condition occurs when the FM manager has not been started by SM during the booting or swact, and raise an exception for this condition is a right approach. Having a new class for the new behavior is a good way to migrate the API changes. Regards, Tao From: Wensley, Barton Sent: Tuesday, February 26, 2019 9:11 AM To: Lin, Shuicheng; MacDonald, Eric; Liu, Tao; Bailey, Henry Albert (Al) Cc: Xie, Cindy; starlingx-discuss@lists.starlingx.io<mailto:starlingx-discuss@lists.starlingx.io> Subject: RE: Improve FM's Python API return data Shuicheng, As per your spreadsheet, there are actually three APIs that currently return ambiguous results (get_fault, get_faults and get_faults_by_id). My view is that you should be updating all six APIs to be consistent and raise an exception if the operation fails (e.g. because the API can't connect to fmmanager). You can avoid impacting the existing APIs by having both a FaultAPIs class and a FaultAPIsV2 class. To avoid code duplication, you would create a FaultAPIsBase class to hold all the common code and subclass from it. Bart From: Lin, Shuicheng [mailto:shuicheng.lin@intel.com] Sent: February 25, 2019 8:59 PM To: MacDonald, Eric; Wensley, Barton; Liu, Tao; Bailey, Henry Albert (Al) Cc: Xie, Cindy; starlingx-discuss@lists.starlingx.io<mailto:starlingx-discuss@lists.starlingx.io> Subject: RE: Improve FM's Python API return data Hi Eric/Tao/Al, Please confirm you agree a new set fault API (FaultAPIs_V2) or not. The attached excel is a simple comparison between original class FaultAPIs and FaultAPIs_V2. For myself, I still prefer to add 2 new API in original class, so other API call could be kept without change. We could have more discuss about the FaultAPIs_V2 design if you all agree to choose it. Best Regards Shuicheng From: MacDonald, Eric [mailto:Eric.MacDonald@windriver.com] Sent: Monday, February 25, 2019 10:23 PM To: Wensley, Barton <Barton.Wensley@windriver.com<mailto:Barton.Wensley@windriver.com>>; Lin, Shuicheng <shuicheng.lin@intel.com<mailto:shuicheng.lin@intel.com>>; Liu, Tao <Tao.Liu@windriver.com<mailto:Tao.Liu@windriver.com>>; Bailey, Henry Albert (Al) <Al.Bailey@windriver.com<mailto:Al.Bailey@windriver.com>> Cc: Xie, Cindy <cindy.xie@intel.com<mailto:cindy.xie@intel.com>>; starlingx-discuss@lists.starlingx.io<mailto:starlingx-discuss@lists.starlingx.io> Subject: RE: Improve FM's Python API return data I see your point Bart. This Jira is an Enhancement Request and fulfilling that should not be done so half way. All the APIs in V2 should be changed to defer exception handling to the caller as apparently that is the pythonic way. From: Wensley, Barton Sent: Monday, February 25, 2019 8:10 AM To: Lin, Shuicheng; MacDonald, Eric; Liu, Tao; Bailey, Henry Albert (Al) Cc: Xie, Cindy; starlingx-discuss@lists.starlingx.io<mailto:starlingx-discuss@lists.starlingx.io> Subject: RE: Improve FM's Python API return data Importance: High I disagree with this approach. Instead of just updating two of the methods, you should be updating all the methods in the FaultAPIs class that could fail in the same way and have them raise an exception. You should create a new FaultAPIs_v2 class for the updated methods to avoid impacting existing users. When a user of this API wants to use the new exceptions, they will start using the new class. Bart From: Lin, Shuicheng [mailto:shuicheng.lin@intel.com] Sent: February 25, 2019 2:58 AM To: MacDonald, Eric; Liu, Tao; Bailey, Henry Albert (Al); Wensley, Barton Cc: Xie, Cindy; starlingx-discuss@lists.starlingx.io<mailto:starlingx-discuss@lists.starlingx.io> Subject: RE: Improve FM's Python API return data Hi all, Based on the comments in patch [0], I will add two new API, get_fault_ex and get_faults_by_id_ex. I will not add a new class, since all other APIs are kept without change. For get_fault_ex and get_faults_by_id_ex: the return value will be alarm list (maybe None). Exception will be thrown if there is error during API execution. [0]: https://review.openstack.org/637655 Best Regards Shuicheng From: Lin, Shuicheng [mailto:shuicheng.lin@intel.com] Sent: Wednesday, February 20, 2019 10:41 AM To: MacDonald, Eric <Eric.MacDonald@windriver.com<mailto:Eric.MacDonald@windriver.com>>; Liu, Tao <Tao.Liu@windriver.com<mailto:Tao.Liu@windriver.com>>; Al.Bailey@windriver.com<mailto:Al.Bailey@windriver.com> Cc: Xie, Cindy <cindy.xie@intel.com<mailto:cindy.xie@intel.com>>; starlingx-discuss@lists.starlingx.io<mailto:starlingx-discuss@lists.starlingx.io> Subject: Re: [Starlingx-discuss] Improve FM's Python API return data Hi Eric, Thanks for the detail comments. In my plan, the return value for get_fault API will be like below: (False, None): there is error in the API execution. (True, None): API execution correctly, and there is no alarm message. (True, Alarm): API execution correctly, and there is alarm message. I think it should fix the issue you have. Is it right? If you agree on this new API return value change, I will implement it. Thanks. Best Regards Shuicheng From: MacDonald, Eric [mailto:Eric.MacDonald@windriver.com] Sent: Tuesday, February 19, 2019 10:35 PM To: Lin, Shuicheng <shuicheng.lin@intel.com<mailto:shuicheng.lin@intel.com>>; Liu, Tao <Tao.Liu@windriver.com<mailto:Tao.Liu@windriver.com>> Cc: Xie, Cindy <cindy.xie@intel.com<mailto:cindy.xie@intel.com>> Subject: RE: Improve FM's Python API return data I took a look and added comments. I don't think this update fixes the issue I have. From: Lin, Shuicheng [mailto:shuicheng.lin@intel.com] Sent: Monday, February 18, 2019 10:09 PM To: MacDonald, Eric; Liu, Tao Cc: Xie, Cindy Subject: RE: Improve FM's Python API return data Importance: High Resend the mail. Best Regards Shuicheng From: Lin, Shuicheng Sent: Tuesday, February 19, 2019 9:56 AM To: Liu, Tao (Wind River) <tao.liu@windriver.com<mailto:tao.liu@windriver.com>>; 'eric.macdonald@windriver.com' <eric.macdonald@windriver.com<mailto:eric.macdonald@windriver.com>> Cc: Xie, Cindy <cindy.xie@intel.com<mailto:cindy.xie@intel.com>> Subject: Improve FM's Python API return data Hi Tao/Eric, For story 2004859<https://storyboard.openstack.org/#!/story/2004859>, I plan to modify the get_fault api return value as a tuple. (True/False, Alarm) Alarm is valid only when the 1st item is True. Sample code is uploaded for you early review: Stx-fault: https://review.openstack.org/637655 Stx-integ: https://review.openstack.org/637656 Could you help review it and share your thought? If you agree on it, I will implement the code and test it. For test, I will try to kill fmManager/modify its listening port to simulate the failure case. For the api call outside of stx-fault, I may try to call it manually to verify. Please share me if there is better verify method. Thanks. Best Regards Shuicheng
Hi Shuicheng, I've been using your new API FaultAPIsV2 and found that it is not behaving the way we discussed or I expected. I looked back on the code update and this behavior seems to have gotten by all of us. My example is for the clear_fault case. I call clear_fault against an alarmID/entity path that there is no alarm raised for but instead of just returning False or None ; it generates an exception. From below , from Bart who I agree with ... "My view is that you should be updating all six APIs to be consistent and raise an exception if the operation fails (e.g. because the API can't connect to fmmanager)." ... I expected to get an exception pretty much only if there was a 'software error' in the API call or it was 'unable to connect to the fm manager'. Not simply if the referenced alarm does not exist.
From your SS my case is the highlighted
[cid:image003.jpg@01D4DF26.A5398AB0] Eric. From: Lin, Shuicheng [mailto:shuicheng.lin@intel.com] Sent: Tuesday, February 26, 2019 7:47 PM To: Liu, Tao; Wensley, Barton; MacDonald, Eric; Bailey, Henry Albert (Al) Cc: Xie, Cindy; starlingx-discuss@lists.starlingx.io Subject: RE: Improve FM's Python API return data Importance: High Hi all, Thanks for the quick confirmation. I will implement the code as Bart's suggestion. Best Regards Shuicheng From: Liu, Tao [mailto:Tao.Liu@windriver.com] Sent: Tuesday, February 26, 2019 10:31 PM To: Wensley, Barton <Barton.Wensley@windriver.com>; Lin, Shuicheng <shuicheng.lin@intel.com>; MacDonald, Eric <Eric.MacDonald@windriver.com>; Bailey, Henry Albert (Al) <Al.Bailey@windriver.com> Cc: Xie, Cindy <cindy.xie@intel.com>; starlingx-discuss@lists.starlingx.io Subject: RE: Improve FM's Python API return data Hi Shuicheng, I agree with Bart's recommendation. The failure condition occurs when the FM manager has not been started by SM during the booting or swact, and raise an exception for this condition is a right approach. Having a new class for the new behavior is a good way to migrate the API changes. Regards, Tao From: Wensley, Barton Sent: Tuesday, February 26, 2019 9:11 AM To: Lin, Shuicheng; MacDonald, Eric; Liu, Tao; Bailey, Henry Albert (Al) Cc: Xie, Cindy; starlingx-discuss@lists.starlingx.io<mailto:starlingx-discuss@lists.starlingx.io> Subject: RE: Improve FM's Python API return data Shuicheng, As per your spreadsheet, there are actually three APIs that currently return ambiguous results (get_fault, get_faults and get_faults_by_id). My view is that you should be updating all six APIs to be consistent and raise an exception if the operation fails (e.g. because the API can't connect to fmmanager). You can avoid impacting the existing APIs by having both a FaultAPIs class and a FaultAPIsV2 class. To avoid code duplication, you would create a FaultAPIsBase class to hold all the common code and subclass from it. Bart From: Lin, Shuicheng [mailto:shuicheng.lin@intel.com] Sent: February 25, 2019 8:59 PM To: MacDonald, Eric; Wensley, Barton; Liu, Tao; Bailey, Henry Albert (Al) Cc: Xie, Cindy; starlingx-discuss@lists.starlingx.io<mailto:starlingx-discuss@lists.starlingx.io> Subject: RE: Improve FM's Python API return data Hi Eric/Tao/Al, Please confirm you agree a new set fault API (FaultAPIs_V2) or not. The attached excel is a simple comparison between original class FaultAPIs and FaultAPIs_V2. For myself, I still prefer to add 2 new API in original class, so other API call could be kept without change. We could have more discuss about the FaultAPIs_V2 design if you all agree to choose it. Best Regards Shuicheng From: MacDonald, Eric [mailto:Eric.MacDonald@windriver.com] Sent: Monday, February 25, 2019 10:23 PM To: Wensley, Barton <Barton.Wensley@windriver.com<mailto:Barton.Wensley@windriver.com>>; Lin, Shuicheng <shuicheng.lin@intel.com<mailto:shuicheng.lin@intel.com>>; Liu, Tao <Tao.Liu@windriver.com<mailto:Tao.Liu@windriver.com>>; Bailey, Henry Albert (Al) <Al.Bailey@windriver.com<mailto:Al.Bailey@windriver.com>> Cc: Xie, Cindy <cindy.xie@intel.com<mailto:cindy.xie@intel.com>>; starlingx-discuss@lists.starlingx.io<mailto:starlingx-discuss@lists.starlingx.io> Subject: RE: Improve FM's Python API return data I see your point Bart. This Jira is an Enhancement Request and fulfilling that should not be done so half way. All the APIs in V2 should be changed to defer exception handling to the caller as apparently that is the pythonic way. From: Wensley, Barton Sent: Monday, February 25, 2019 8:10 AM To: Lin, Shuicheng; MacDonald, Eric; Liu, Tao; Bailey, Henry Albert (Al) Cc: Xie, Cindy; starlingx-discuss@lists.starlingx.io<mailto:starlingx-discuss@lists.starlingx.io> Subject: RE: Improve FM's Python API return data Importance: High I disagree with this approach. Instead of just updating two of the methods, you should be updating all the methods in the FaultAPIs class that could fail in the same way and have them raise an exception. You should create a new FaultAPIs_v2 class for the updated methods to avoid impacting existing users. When a user of this API wants to use the new exceptions, they will start using the new class. Bart From: Lin, Shuicheng [mailto:shuicheng.lin@intel.com] Sent: February 25, 2019 2:58 AM To: MacDonald, Eric; Liu, Tao; Bailey, Henry Albert (Al); Wensley, Barton Cc: Xie, Cindy; starlingx-discuss@lists.starlingx.io<mailto:starlingx-discuss@lists.starlingx.io> Subject: RE: Improve FM's Python API return data Hi all, Based on the comments in patch [0], I will add two new API, get_fault_ex and get_faults_by_id_ex. I will not add a new class, since all other APIs are kept without change. For get_fault_ex and get_faults_by_id_ex: the return value will be alarm list (maybe None). Exception will be thrown if there is error during API execution. [0]: https://review.openstack.org/637655 Best Regards Shuicheng From: Lin, Shuicheng [mailto:shuicheng.lin@intel.com] Sent: Wednesday, February 20, 2019 10:41 AM To: MacDonald, Eric <Eric.MacDonald@windriver.com<mailto:Eric.MacDonald@windriver.com>>; Liu, Tao <Tao.Liu@windriver.com<mailto:Tao.Liu@windriver.com>>; Al.Bailey@windriver.com<mailto:Al.Bailey@windriver.com> Cc: Xie, Cindy <cindy.xie@intel.com<mailto:cindy.xie@intel.com>>; starlingx-discuss@lists.starlingx.io<mailto:starlingx-discuss@lists.starlingx.io> Subject: Re: [Starlingx-discuss] Improve FM's Python API return data Hi Eric, Thanks for the detail comments. In my plan, the return value for get_fault API will be like below: (False, None): there is error in the API execution. (True, None): API execution correctly, and there is no alarm message. (True, Alarm): API execution correctly, and there is alarm message. I think it should fix the issue you have. Is it right? If you agree on this new API return value change, I will implement it. Thanks. Best Regards Shuicheng From: MacDonald, Eric [mailto:Eric.MacDonald@windriver.com] Sent: Tuesday, February 19, 2019 10:35 PM To: Lin, Shuicheng <shuicheng.lin@intel.com<mailto:shuicheng.lin@intel.com>>; Liu, Tao <Tao.Liu@windriver.com<mailto:Tao.Liu@windriver.com>> Cc: Xie, Cindy <cindy.xie@intel.com<mailto:cindy.xie@intel.com>> Subject: RE: Improve FM's Python API return data I took a look and added comments. I don't think this update fixes the issue I have. From: Lin, Shuicheng [mailto:shuicheng.lin@intel.com] Sent: Monday, February 18, 2019 10:09 PM To: MacDonald, Eric; Liu, Tao Cc: Xie, Cindy Subject: RE: Improve FM's Python API return data Importance: High Resend the mail. Best Regards Shuicheng From: Lin, Shuicheng Sent: Tuesday, February 19, 2019 9:56 AM To: Liu, Tao (Wind River) <tao.liu@windriver.com<mailto:tao.liu@windriver.com>>; 'eric.macdonald@windriver.com' <eric.macdonald@windriver.com<mailto:eric.macdonald@windriver.com>> Cc: Xie, Cindy <cindy.xie@intel.com<mailto:cindy.xie@intel.com>> Subject: Improve FM's Python API return data Hi Tao/Eric, For story 2004859<https://storyboard.openstack.org/#!/story/2004859>, I plan to modify the get_fault api return value as a tuple. (True/False, Alarm) Alarm is valid only when the 1st item is True. Sample code is uploaded for you early review: Stx-fault: https://review.openstack.org/637655 Stx-integ: https://review.openstack.org/637656 Could you help review it and share your thought? If you agree on it, I will implement the code and test it. For test, I will try to kill fmManager/modify its listening port to simulate the failure case. For the api call outside of stx-fault, I may try to call it manually to verify. Please share me if there is better verify method. Thanks. Best Regards Shuicheng
Hi Eric, Thanks for using the new API. :) I will update code to take "clear non exist alarm" as normal case (no exception) as your suggestion. Another thing is, I don't think we need return value for clear_fault/clear_all API. If we want return value for clear_fault/clear_all, then we could define return value as below: True: alarm is cleared False: alarm is not found. Exception: there is operation failure. Which version do you prefer? Thanks. I created below LP issue to track it: https://bugs.launchpad.net/starlingx/+bug/1821112 Best Regards Shuicheng From: MacDonald, Eric [mailto:Eric.MacDonald@windriver.com] Sent: Thursday, March 21, 2019 2:10 AM To: Lin, Shuicheng <shuicheng.lin@intel.com>; Liu, Tao <Tao.Liu@windriver.com>; Wensley, Barton <Barton.Wensley@windriver.com>; Bailey, Henry Albert (Al) <Al.Bailey@windriver.com> Cc: Xie, Cindy <cindy.xie@intel.com>; starlingx-discuss@lists.starlingx.io; Khalil, Ghada <Ghada.Khalil@windriver.com> Subject: RE: Improve FM's Python API return data Hi Shuicheng, I've been using your new API FaultAPIsV2 and found that it is not behaving the way we discussed or I expected. I looked back on the code update and this behavior seems to have gotten by all of us. My example is for the clear_fault case. I call clear_fault against an alarmID/entity path that there is no alarm raised for but instead of just returning False or None ; it generates an exception. From below , from Bart who I agree with ... "My view is that you should be updating all six APIs to be consistent and raise an exception if the operation fails (e.g. because the API can't connect to fmmanager)." ... I expected to get an exception pretty much only if there was a 'software error' in the API call or it was 'unable to connect to the fm manager'. Not simply if the referenced alarm does not exist.
From your SS my case is the highlighted
[cid:image001.jpg@01D4DFC0.AC313D50] Eric. From: Lin, Shuicheng [mailto:shuicheng.lin@intel.com] Sent: Tuesday, February 26, 2019 7:47 PM To: Liu, Tao; Wensley, Barton; MacDonald, Eric; Bailey, Henry Albert (Al) Cc: Xie, Cindy; starlingx-discuss@lists.starlingx.io<mailto:starlingx-discuss@lists.starlingx.io> Subject: RE: Improve FM's Python API return data Importance: High Hi all, Thanks for the quick confirmation. I will implement the code as Bart's suggestion. Best Regards Shuicheng From: Liu, Tao [mailto:Tao.Liu@windriver.com] Sent: Tuesday, February 26, 2019 10:31 PM To: Wensley, Barton <Barton.Wensley@windriver.com<mailto:Barton.Wensley@windriver.com>>; Lin, Shuicheng <shuicheng.lin@intel.com<mailto:shuicheng.lin@intel.com>>; MacDonald, Eric <Eric.MacDonald@windriver.com<mailto:Eric.MacDonald@windriver.com>>; Bailey, Henry Albert (Al) <Al.Bailey@windriver.com<mailto:Al.Bailey@windriver.com>> Cc: Xie, Cindy <cindy.xie@intel.com<mailto:cindy.xie@intel.com>>; starlingx-discuss@lists.starlingx.io<mailto:starlingx-discuss@lists.starlingx.io> Subject: RE: Improve FM's Python API return data Hi Shuicheng, I agree with Bart's recommendation. The failure condition occurs when the FM manager has not been started by SM during the booting or swact, and raise an exception for this condition is a right approach. Having a new class for the new behavior is a good way to migrate the API changes. Regards, Tao From: Wensley, Barton Sent: Tuesday, February 26, 2019 9:11 AM To: Lin, Shuicheng; MacDonald, Eric; Liu, Tao; Bailey, Henry Albert (Al) Cc: Xie, Cindy; starlingx-discuss@lists.starlingx.io<mailto:starlingx-discuss@lists.starlingx.io> Subject: RE: Improve FM's Python API return data Shuicheng, As per your spreadsheet, there are actually three APIs that currently return ambiguous results (get_fault, get_faults and get_faults_by_id). My view is that you should be updating all six APIs to be consistent and raise an exception if the operation fails (e.g. because the API can't connect to fmmanager). You can avoid impacting the existing APIs by having both a FaultAPIs class and a FaultAPIsV2 class. To avoid code duplication, you would create a FaultAPIsBase class to hold all the common code and subclass from it. Bart From: Lin, Shuicheng [mailto:shuicheng.lin@intel.com] Sent: February 25, 2019 8:59 PM To: MacDonald, Eric; Wensley, Barton; Liu, Tao; Bailey, Henry Albert (Al) Cc: Xie, Cindy; starlingx-discuss@lists.starlingx.io<mailto:starlingx-discuss@lists.starlingx.io> Subject: RE: Improve FM's Python API return data Hi Eric/Tao/Al, Please confirm you agree a new set fault API (FaultAPIs_V2) or not. The attached excel is a simple comparison between original class FaultAPIs and FaultAPIs_V2. For myself, I still prefer to add 2 new API in original class, so other API call could be kept without change. We could have more discuss about the FaultAPIs_V2 design if you all agree to choose it. Best Regards Shuicheng From: MacDonald, Eric [mailto:Eric.MacDonald@windriver.com] Sent: Monday, February 25, 2019 10:23 PM To: Wensley, Barton <Barton.Wensley@windriver.com<mailto:Barton.Wensley@windriver.com>>; Lin, Shuicheng <shuicheng.lin@intel.com<mailto:shuicheng.lin@intel.com>>; Liu, Tao <Tao.Liu@windriver.com<mailto:Tao.Liu@windriver.com>>; Bailey, Henry Albert (Al) <Al.Bailey@windriver.com<mailto:Al.Bailey@windriver.com>> Cc: Xie, Cindy <cindy.xie@intel.com<mailto:cindy.xie@intel.com>>; starlingx-discuss@lists.starlingx.io<mailto:starlingx-discuss@lists.starlingx.io> Subject: RE: Improve FM's Python API return data I see your point Bart. This Jira is an Enhancement Request and fulfilling that should not be done so half way. All the APIs in V2 should be changed to defer exception handling to the caller as apparently that is the pythonic way. From: Wensley, Barton Sent: Monday, February 25, 2019 8:10 AM To: Lin, Shuicheng; MacDonald, Eric; Liu, Tao; Bailey, Henry Albert (Al) Cc: Xie, Cindy; starlingx-discuss@lists.starlingx.io<mailto:starlingx-discuss@lists.starlingx.io> Subject: RE: Improve FM's Python API return data Importance: High I disagree with this approach. Instead of just updating two of the methods, you should be updating all the methods in the FaultAPIs class that could fail in the same way and have them raise an exception. You should create a new FaultAPIs_v2 class for the updated methods to avoid impacting existing users. When a user of this API wants to use the new exceptions, they will start using the new class. Bart From: Lin, Shuicheng [mailto:shuicheng.lin@intel.com] Sent: February 25, 2019 2:58 AM To: MacDonald, Eric; Liu, Tao; Bailey, Henry Albert (Al); Wensley, Barton Cc: Xie, Cindy; starlingx-discuss@lists.starlingx.io<mailto:starlingx-discuss@lists.starlingx.io> Subject: RE: Improve FM's Python API return data Hi all, Based on the comments in patch [0], I will add two new API, get_fault_ex and get_faults_by_id_ex. I will not add a new class, since all other APIs are kept without change. For get_fault_ex and get_faults_by_id_ex: the return value will be alarm list (maybe None). Exception will be thrown if there is error during API execution. [0]: https://review.openstack.org/637655 Best Regards Shuicheng From: Lin, Shuicheng [mailto:shuicheng.lin@intel.com] Sent: Wednesday, February 20, 2019 10:41 AM To: MacDonald, Eric <Eric.MacDonald@windriver.com<mailto:Eric.MacDonald@windriver.com>>; Liu, Tao <Tao.Liu@windriver.com<mailto:Tao.Liu@windriver.com>>; Al.Bailey@windriver.com<mailto:Al.Bailey@windriver.com> Cc: Xie, Cindy <cindy.xie@intel.com<mailto:cindy.xie@intel.com>>; starlingx-discuss@lists.starlingx.io<mailto:starlingx-discuss@lists.starlingx.io> Subject: Re: [Starlingx-discuss] Improve FM's Python API return data Hi Eric, Thanks for the detail comments. In my plan, the return value for get_fault API will be like below: (False, None): there is error in the API execution. (True, None): API execution correctly, and there is no alarm message. (True, Alarm): API execution correctly, and there is alarm message. I think it should fix the issue you have. Is it right? If you agree on this new API return value change, I will implement it. Thanks. Best Regards Shuicheng From: MacDonald, Eric [mailto:Eric.MacDonald@windriver.com] Sent: Tuesday, February 19, 2019 10:35 PM To: Lin, Shuicheng <shuicheng.lin@intel.com<mailto:shuicheng.lin@intel.com>>; Liu, Tao <Tao.Liu@windriver.com<mailto:Tao.Liu@windriver.com>> Cc: Xie, Cindy <cindy.xie@intel.com<mailto:cindy.xie@intel.com>> Subject: RE: Improve FM's Python API return data I took a look and added comments. I don't think this update fixes the issue I have. From: Lin, Shuicheng [mailto:shuicheng.lin@intel.com] Sent: Monday, February 18, 2019 10:09 PM To: MacDonald, Eric; Liu, Tao Cc: Xie, Cindy Subject: RE: Improve FM's Python API return data Importance: High Resend the mail. Best Regards Shuicheng From: Lin, Shuicheng Sent: Tuesday, February 19, 2019 9:56 AM To: Liu, Tao (Wind River) <tao.liu@windriver.com<mailto:tao.liu@windriver.com>>; 'eric.macdonald@windriver.com' <eric.macdonald@windriver.com<mailto:eric.macdonald@windriver.com>> Cc: Xie, Cindy <cindy.xie@intel.com<mailto:cindy.xie@intel.com>> Subject: Improve FM's Python API return data Hi Tao/Eric, For story 2004859<https://storyboard.openstack.org/#!/story/2004859>, I plan to modify the get_fault api return value as a tuple. (True/False, Alarm) Alarm is valid only when the 1st item is True. Sample code is uploaded for you early review: Stx-fault: https://review.openstack.org/637655 Stx-integ: https://review.openstack.org/637656 Could you help review it and share your thought? If you agree on it, I will implement the code and test it. For test, I will try to kill fmManager/modify its listening port to simulate the failure case. For the api call outside of stx-fault, I may try to call it manually to verify. Please share me if there is better verify method. Thanks. Best Regards Shuicheng
Hi Shuicheng, First of all, thank you for supporting this change for us. It's much appreciated. Yes, please go with (what was in your spreadsheet) True: alarm is cleared False: alarm is not found. Exception: there is operation failure. Please apply that change to the other new APIs as well. Looking at the code now more closely we need to ensure that the 'clear_fault', 'get_fault','clear_all' , 'get_faults' and 'get_faults_by_id' all follow this approach. Interestingly I don't see the get_faults_by_id returning an exception in my code in the case where there are none found. It is behaving properly but looking at the code I can only assume that fm_core.get_by_aid is returning None rather than False which would explain it. Please test 'each' the APIs with the following conditions ... * With applicable alarm present or success path * With applicable alarm not present. * With FM main process not running (you can do this by runnin the operations on a compute while there is no controller up.) * With an (software) error API call (like using an invalid alarmID) Eric. From: Lin, Shuicheng [mailto:shuicheng.lin@intel.com] Sent: Wednesday, March 20, 2019 9:33 PM To: MacDonald, Eric; Liu, Tao; Wensley, Barton; Bailey, Henry Albert (Al) Cc: Xie, Cindy; starlingx-discuss@lists.starlingx.io; Khalil, Ghada Subject: RE: Improve FM's Python API return data Importance: High Hi Eric, Thanks for using the new API. :) I will update code to take "clear non exist alarm" as normal case (no exception) as your suggestion. Another thing is, I don't think we need return value for clear_fault/clear_all API. If we want return value for clear_fault/clear_all, then we could define return value as below: True: alarm is cleared False: alarm is not found. Exception: there is operation failure. Which version do you prefer? Thanks. I created below LP issue to track it: https://bugs.launchpad.net/starlingx/+bug/1821112 Best Regards Shuicheng From: MacDonald, Eric [mailto:Eric.MacDonald@windriver.com] Sent: Thursday, March 21, 2019 2:10 AM To: Lin, Shuicheng <shuicheng.lin@intel.com>; Liu, Tao <Tao.Liu@windriver.com>; Wensley, Barton <Barton.Wensley@windriver.com>; Bailey, Henry Albert (Al) <Al.Bailey@windriver.com> Cc: Xie, Cindy <cindy.xie@intel.com>; starlingx-discuss@lists.starlingx.io; Khalil, Ghada <Ghada.Khalil@windriver.com> Subject: RE: Improve FM's Python API return data Hi Shuicheng, I've been using your new API FaultAPIsV2 and found that it is not behaving the way we discussed or I expected. I looked back on the code update and this behavior seems to have gotten by all of us. My example is for the clear_fault case. I call clear_fault against an alarmID/entity path that there is no alarm raised for but instead of just returning False or None ; it generates an exception. From below , from Bart who I agree with ... "My view is that you should be updating all six APIs to be consistent and raise an exception if the operation fails (e.g. because the API can't connect to fmmanager)." ... I expected to get an exception pretty much only if there was a 'software error' in the API call or it was 'unable to connect to the fm manager'. Not simply if the referenced alarm does not exist.
From your SS my case is the highlighted
[cid:image001.jpg@01D4DFC0.AC313D50] Eric. From: Lin, Shuicheng [mailto:shuicheng.lin@intel.com] Sent: Tuesday, February 26, 2019 7:47 PM To: Liu, Tao; Wensley, Barton; MacDonald, Eric; Bailey, Henry Albert (Al) Cc: Xie, Cindy; starlingx-discuss@lists.starlingx.io<mailto:starlingx-discuss@lists.starlingx.io> Subject: RE: Improve FM's Python API return data Importance: High Hi all, Thanks for the quick confirmation. I will implement the code as Bart's suggestion. Best Regards Shuicheng From: Liu, Tao [mailto:Tao.Liu@windriver.com] Sent: Tuesday, February 26, 2019 10:31 PM To: Wensley, Barton <Barton.Wensley@windriver.com<mailto:Barton.Wensley@windriver.com>>; Lin, Shuicheng <shuicheng.lin@intel.com<mailto:shuicheng.lin@intel.com>>; MacDonald, Eric <Eric.MacDonald@windriver.com<mailto:Eric.MacDonald@windriver.com>>; Bailey, Henry Albert (Al) <Al.Bailey@windriver.com<mailto:Al.Bailey@windriver.com>> Cc: Xie, Cindy <cindy.xie@intel.com<mailto:cindy.xie@intel.com>>; starlingx-discuss@lists.starlingx.io<mailto:starlingx-discuss@lists.starlingx.io> Subject: RE: Improve FM's Python API return data Hi Shuicheng, I agree with Bart's recommendation. The failure condition occurs when the FM manager has not been started by SM during the booting or swact, and raise an exception for this condition is a right approach. Having a new class for the new behavior is a good way to migrate the API changes. Regards, Tao From: Wensley, Barton Sent: Tuesday, February 26, 2019 9:11 AM To: Lin, Shuicheng; MacDonald, Eric; Liu, Tao; Bailey, Henry Albert (Al) Cc: Xie, Cindy; starlingx-discuss@lists.starlingx.io<mailto:starlingx-discuss@lists.starlingx.io> Subject: RE: Improve FM's Python API return data Shuicheng, As per your spreadsheet, there are actually three APIs that currently return ambiguous results (get_fault, get_faults and get_faults_by_id). My view is that you should be updating all six APIs to be consistent and raise an exception if the operation fails (e.g. because the API can't connect to fmmanager). You can avoid impacting the existing APIs by having both a FaultAPIs class and a FaultAPIsV2 class. To avoid code duplication, you would create a FaultAPIsBase class to hold all the common code and subclass from it. Bart From: Lin, Shuicheng [mailto:shuicheng.lin@intel.com] Sent: February 25, 2019 8:59 PM To: MacDonald, Eric; Wensley, Barton; Liu, Tao; Bailey, Henry Albert (Al) Cc: Xie, Cindy; starlingx-discuss@lists.starlingx.io<mailto:starlingx-discuss@lists.starlingx.io> Subject: RE: Improve FM's Python API return data Hi Eric/Tao/Al, Please confirm you agree a new set fault API (FaultAPIs_V2) or not. The attached excel is a simple comparison between original class FaultAPIs and FaultAPIs_V2. For myself, I still prefer to add 2 new API in original class, so other API call could be kept without change. We could have more discuss about the FaultAPIs_V2 design if you all agree to choose it. Best Regards Shuicheng From: MacDonald, Eric [mailto:Eric.MacDonald@windriver.com] Sent: Monday, February 25, 2019 10:23 PM To: Wensley, Barton <Barton.Wensley@windriver.com<mailto:Barton.Wensley@windriver.com>>; Lin, Shuicheng <shuicheng.lin@intel.com<mailto:shuicheng.lin@intel.com>>; Liu, Tao <Tao.Liu@windriver.com<mailto:Tao.Liu@windriver.com>>; Bailey, Henry Albert (Al) <Al.Bailey@windriver.com<mailto:Al.Bailey@windriver.com>> Cc: Xie, Cindy <cindy.xie@intel.com<mailto:cindy.xie@intel.com>>; starlingx-discuss@lists.starlingx.io<mailto:starlingx-discuss@lists.starlingx.io> Subject: RE: Improve FM's Python API return data I see your point Bart. This Jira is an Enhancement Request and fulfilling that should not be done so half way. All the APIs in V2 should be changed to defer exception handling to the caller as apparently that is the pythonic way. From: Wensley, Barton Sent: Monday, February 25, 2019 8:10 AM To: Lin, Shuicheng; MacDonald, Eric; Liu, Tao; Bailey, Henry Albert (Al) Cc: Xie, Cindy; starlingx-discuss@lists.starlingx.io<mailto:starlingx-discuss@lists.starlingx.io> Subject: RE: Improve FM's Python API return data Importance: High I disagree with this approach. Instead of just updating two of the methods, you should be updating all the methods in the FaultAPIs class that could fail in the same way and have them raise an exception. You should create a new FaultAPIs_v2 class for the updated methods to avoid impacting existing users. When a user of this API wants to use the new exceptions, they will start using the new class. Bart From: Lin, Shuicheng [mailto:shuicheng.lin@intel.com] Sent: February 25, 2019 2:58 AM To: MacDonald, Eric; Liu, Tao; Bailey, Henry Albert (Al); Wensley, Barton Cc: Xie, Cindy; starlingx-discuss@lists.starlingx.io<mailto:starlingx-discuss@lists.starlingx.io> Subject: RE: Improve FM's Python API return data Hi all, Based on the comments in patch [0], I will add two new API, get_fault_ex and get_faults_by_id_ex. I will not add a new class, since all other APIs are kept without change. For get_fault_ex and get_faults_by_id_ex: the return value will be alarm list (maybe None). Exception will be thrown if there is error during API execution. [0]: https://review.openstack.org/637655 Best Regards Shuicheng From: Lin, Shuicheng [mailto:shuicheng.lin@intel.com] Sent: Wednesday, February 20, 2019 10:41 AM To: MacDonald, Eric <Eric.MacDonald@windriver.com<mailto:Eric.MacDonald@windriver.com>>; Liu, Tao <Tao.Liu@windriver.com<mailto:Tao.Liu@windriver.com>>; Al.Bailey@windriver.com<mailto:Al.Bailey@windriver.com> Cc: Xie, Cindy <cindy.xie@intel.com<mailto:cindy.xie@intel.com>>; starlingx-discuss@lists.starlingx.io<mailto:starlingx-discuss@lists.starlingx.io> Subject: Re: [Starlingx-discuss] Improve FM's Python API return data Hi Eric, Thanks for the detail comments. In my plan, the return value for get_fault API will be like below: (False, None): there is error in the API execution. (True, None): API execution correctly, and there is no alarm message. (True, Alarm): API execution correctly, and there is alarm message. I think it should fix the issue you have. Is it right? If you agree on this new API return value change, I will implement it. Thanks. Best Regards Shuicheng From: MacDonald, Eric [mailto:Eric.MacDonald@windriver.com] Sent: Tuesday, February 19, 2019 10:35 PM To: Lin, Shuicheng <shuicheng.lin@intel.com<mailto:shuicheng.lin@intel.com>>; Liu, Tao <Tao.Liu@windriver.com<mailto:Tao.Liu@windriver.com>> Cc: Xie, Cindy <cindy.xie@intel.com<mailto:cindy.xie@intel.com>> Subject: RE: Improve FM's Python API return data I took a look and added comments. I don't think this update fixes the issue I have. From: Lin, Shuicheng [mailto:shuicheng.lin@intel.com] Sent: Monday, February 18, 2019 10:09 PM To: MacDonald, Eric; Liu, Tao Cc: Xie, Cindy Subject: RE: Improve FM's Python API return data Importance: High Resend the mail. Best Regards Shuicheng From: Lin, Shuicheng Sent: Tuesday, February 19, 2019 9:56 AM To: Liu, Tao (Wind River) <tao.liu@windriver.com<mailto:tao.liu@windriver.com>>; 'eric.macdonald@windriver.com' <eric.macdonald@windriver.com<mailto:eric.macdonald@windriver.com>> Cc: Xie, Cindy <cindy.xie@intel.com<mailto:cindy.xie@intel.com>> Subject: Improve FM's Python API return data Hi Tao/Eric, For story 2004859<https://storyboard.openstack.org/#!/story/2004859>, I plan to modify the get_fault api return value as a tuple. (True/False, Alarm) Alarm is valid only when the 1st item is True. Sample code is uploaded for you early review: Stx-fault: https://review.openstack.org/637655 Stx-integ: https://review.openstack.org/637656 Could you help review it and share your thought? If you agree on it, I will implement the code and test it. For test, I will try to kill fmManager/modify its listening port to simulate the failure case. For the api call outside of stx-fault, I may try to call it manually to verify. Please share me if there is better verify method. Thanks. Best Regards Shuicheng
participants (5)
-
Bailey, Henry Albert (Al)
-
Lin, Shuicheng
-
Liu, Tao
-
MacDonald, Eric
-
Wensley, Barton