Skip to content

Commit

Permalink
Fix SAI adapter exception due to POE PSE attribute conflicting with v…
Browse files Browse the repository at this point in the history
…ariables in sai_adapter.py. (#2011)

After the recent POE PSE API update, the SAI adapter python script will fail on launch with the exception below:

byte-compiling /dash/dash-pipeline/SAI/saithrift/../rpc/usr/local/lib/python3.10/site-packages/sai_thrift/sai_adapter.py to sai_adapter.cpython-310.pyc
  File "/dash/dash-pipeline/SAI/saithrift/../rpc/usr/local/lib/python3.10/site-packages/sai_thrift/sai_adapter.py", line 37000
    global status
    ^^^^^^^^^^^^^
SyntaxError: name 'status' is parameter and global
The reason is because the POE PSE API has a attribute with name STATUS:

/**
 * @brief POE PSE attributes
 */
typedef enum _sai_poe_pse_attr_t
{
    ...

    /**
     * @brief Status of the PSE
     *
     * @type sai_poe_pse_status_t
     * @flags READ_ONLY
     */
    SAI_POE_PSE_ATTR_STATUS,

    ...
} sai_poe_pse_attr_t;
This causes the status parameter being generated in the SAI thrift adapter, and conflicting with the variable global status:

Fix

Since it is not ideal to change the API name, as it might break any code rely on it. This commit updates the template for generating the adapter code with 2 changes:

Rename the status global variable into sai_status.
Add a fallback __getattr__ function (called after no attribute is found), which ensures any existing code that gets the status variable will not break, such as adapter.status.


Signed-off-by: r12f <r12f.code@gmail.com>
  • Loading branch information
r12f authored and Kishore Gummadidala committed Sep 17, 2024
1 parent aebfa31 commit ec29e0d
Showing 1 changed file with 25 additions and 18 deletions.
43 changes: 25 additions & 18 deletions meta/templates/sai_adapter.py.tt
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,7 @@
[%- ELSIF function.operation == 'stats' %]
Dict[str, [% function.rpc_return.type.subtype.python_name %]]: stats
[%- ELSIF function.rpc_return.type.name == 'void' AND NOT function.rpc_return.is_list %]
status: the error code
sai_status: the error code
[%- ELSE %]
[% function.rpc_return.type.python_name %]: [% function.rpc_return.name %]
[%- END -%]
Expand All @@ -34,7 +34,7 @@
[%- ELSIF function.rpc_return.type.name == 'void' AND NOT function.rpc_return.is_list %]

Returns:
status: the error code
sai_status: the error code
[%- END -%]
[%- END %]

Expand Down Expand Up @@ -190,10 +190,10 @@ client.[% function.thrift_name %](

[%- BLOCK catch_exception -%]
except sai_thrift_exception as e:
status = e.status
if SKIP_TEST_ON_EXPECTED_ERROR and status in EXPECTED_ERROR_CODE:
sai_status = e.status
if SKIP_TEST_ON_EXPECTED_ERROR and sai_status in EXPECTED_ERROR_CODE:
reason = "SkipTest on expected error. [% function.thrift_name %] with errorcode: {} error: {}".format(
status, e)
sai_status, e)
print(reason)
testutils.skipped_test_count=1
raise SkipTest(reason)
Expand All @@ -207,7 +207,7 @@ client.[% function.thrift_name %](
[%- ELSIF function.operation == 'create' %]
pass
[%- ELSE %]
return status
return sai_status
[%- END %]
else:
raise e
Expand Down Expand Up @@ -275,8 +275,8 @@ client.[% function.thrift_name %](
[%- # For 'set attr' function we just do call the funtion for first argument -%]
[%- IF function.operation == 'set' -%]

global status
status = SAI_STATUS_SUCCESS
global sai_status
sai_status = SAI_STATUS_SUCCESS


[%- WRAPPER try -%]
Expand Down Expand Up @@ -413,11 +413,11 @@ client.[% function.thrift_name %](
[%- ######################################################################## -%]

[%- BLOCK return_from_empty_function -%]
global status
status = SAI_STATUS_NOT_SUPPORTED
if SKIP_TEST_ON_EXPECTED_ERROR and status in EXPECTED_ERROR_CODE:
global sai_status
sai_status = SAI_STATUS_NOT_SUPPORTED
if SKIP_TEST_ON_EXPECTED_ERROR and sai_status in EXPECTED_ERROR_CODE:
reason = "SkipTest on expected error. [% function.name %] with errorcode: {} error: {}".format(
status, e)
sai_status, e)
print(reason)
testutils.skipped_test_count=1
raise SkipTest(reason)
Expand All @@ -426,12 +426,12 @@ client.[% function.thrift_name %](
[%- IF function.operation == 'create' AND NOT function.rpc_return.is_list %]
return SAI_NULL_OBJECT_ID
[%- ELSIF function.operation != 'get' AND function.operation != 'stats' AND function.rpc_return.type.name == 'void' %]
return status
return sai_status
[%- ELSE %]
return None
[%- END %]
else:
raise sai_thrift_exception(status)
raise sai_thrift_exception(sai_status)
[%- END -%]

[%- ######################################################################## -%]
Expand All @@ -454,8 +454,8 @@ client.[% function.thrift_name %](
[%- # Now, call the thrift function -%]
[%- IF function.operation != 'set' -%]

global status
status = SAI_STATUS_SUCCESS
global sai_status
sai_status = SAI_STATUS_SUCCESS


[%- WRAPPER try -%]
Expand All @@ -468,7 +468,7 @@ client.[% function.thrift_name %](

[%- # Return a status only if a function does not, and does not return something else -%]
[%- IF function.operation != 'get' AND function.operation != 'stats' AND function.rpc_return.type.name == 'void' %]
return status
return sai_status

[%- END -%]

Expand Down Expand Up @@ -523,7 +523,14 @@ CATCH_EXCEPTIONS = True
EXPECTED_ERROR_CODE = [-2]
# Skip test when hitting an expected error
SKIP_TEST_ON_EXPECTED_ERROR = True
status = 0
sai_status = 0

def __getattr__(name):
if name == 'status':
global sai_status
return sai_status
else:
raise AttributeError(f'module {__name__} has no attribute {name}')

[%- PROCESS dev_utils IF dev_utils -%]
[%- PROCESS invocation_logger IF adapter_logger -%]
Expand Down

0 comments on commit ec29e0d

Please sign in to comment.