Skip to content

Conversation

@yma955
Copy link
Member

@yma955 yma955 commented May 28, 2025

No description provided.

@coveralls
Copy link

coveralls commented May 28, 2025

Pull Request Test Coverage Report for Build 15289854030

Details

  • 1 of 8 (12.5%) changed or added relevant lines in 1 file are covered.
  • No unchanged relevant lines lost coverage.
  • Overall coverage decreased (-0.1%) to 63.818%

Changes Missing Coverage Covered Lines Changed/Added Lines %
charon/pkgs/radas_signature_handler.py 1 8 12.5%
Totals Coverage Status
Change from base Build 15254309416: -0.1%
Covered Lines: 2039
Relevant Lines: 3195

💛 - Coveralls

@yma955 yma955 requested a review from ligangty May 28, 2025 01:53
@yma955 yma955 force-pushed the add-request-id branch 3 times, most recently from ebdb929 to c970571 Compare May 28, 2025 02:06
Copy link
Member

@ligangty ligangty left a comment

Choose a reason for hiding this comment

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

Looks good.

@ligangty
Copy link
Member

ligangty commented May 28, 2025

@yma96 Actually we still miss one part of the logic. If you check the tech design, you will see that the response file will give fields "signing_status" and "errors". That means it will possibly get errors in radas side. We should cover this part of handling. What I'm thinking here is, we should have a field in Receiver to tell if the response is successful or failed, and field to record the errors. Then in main process we can control based on these fields. WDYT?
If you think this is good, you can use a separate PR to handle this.

@yma955
Copy link
Member Author

yma955 commented May 29, 2025

@ligangty That sounds good, I'll have another PR to handle the response, will merge this first, thanks.

@yma955 yma955 merged commit bf01903 into Commonjava:main May 29, 2025
17 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants