Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix(agw): Modified code to fix the issue 14244 #14327

Merged
merged 2 commits into from
Nov 1, 2022

Conversation

rsarwad
Copy link
Contributor

@rsarwad rsarwad commented Oct 31, 2022

Summary

Closes #14244

Modified code to read enb_name in character array format as expected by API, set_guage(). Since enb_name, read from proto files was in string format was resulting into serialization and de-serialization errors.
The PR provides fixes for the issue, #14244

Test Plan

Executed s1ap sanity test suite and unit test cases

Signed-off-by: Rashmi <rashmi.sarwad@radisys.com>
@rsarwad rsarwad requested review from a team and pruthvihebbani October 31, 2022 09:00
@pull-request-size pull-request-size bot added the size/XS Denotes a PR that changes 0-9 lines. label Oct 31, 2022
@github-actions
Copy link
Contributor

Thanks for opening a PR! 💯

A couple initial guidelines

Howto

  • Reviews. The "Reviewers" listed for this PR are the Magma maintainers who will shepherd it.
  • Checks. All required CI checks must pass before merge.
  • Merge. Once approved and passing CI checks, use the ready2merge label to indicate the maintainers can merge your PR.

More info

Please take a moment to read through the Magma project's

If this is your first Magma PR, also consider reading

@github-actions github-actions bot added the component: agw Access gateway-related issue label Oct 31, 2022
@rsarwad rsarwad requested a review from crasu October 31, 2022 09:01
Copy link
Contributor

@crasu crasu left a comment

Choose a reason for hiding this comment

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

Code change looks good. Could you run the lte integ tests with this change from your repo to prove the error message is gone.

…4244

Signed-off-by: Rashmi <rashmi.sarwad@radisys.com>
@rsarwad
Copy link
Contributor Author

rsarwad commented Oct 31, 2022

Code change looks good. Could you run the lte integ tests with this change from your repo to prove the error message is gone.

Yes @crasu Locally verified that the protobuf error messages are not seen. But while I was self reviewing I got to know that similar changes are needed at some other places. Modified the code and again kept for executing "lte integ_tests". Let you know once that is completed

@rsarwad
Copy link
Contributor Author

rsarwad commented Oct 31, 2022

Code change looks good. Could you run the lte integ tests with this change from your repo to prove the error message is gone.

Yes @crasu Locally verified that the protobuf error messages are not seen. But while I was self reviewing I got to know that similar changes are needed at some other places. Modified the code and again kept for executing "lte integ_tests". Let you know once that is completed

@crasu, "lte integ_tests" completed successfully in local setup

Copy link
Collaborator

@ssanadhya ssanadhya left a comment

Choose a reason for hiding this comment

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

Lgtm

@ssanadhya ssanadhya merged commit 8123305 into magma:master Nov 1, 2022
lucasgonze pushed a commit to lucasgonze/magma that referenced this pull request Feb 29, 2024
Signed-off-by: Rashmi <rashmi.sarwad@radisys.com>
edaspb pushed a commit to edaspb/magma that referenced this pull request Mar 22, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
component: agw Access gateway-related issue size/XS Denotes a PR that changes 0-9 lines.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

De/Serialization errors during running integ tests
4 participants