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

Async->Sync revert #1286

Merged
merged 12 commits into from
Nov 13, 2023
Merged

Async->Sync revert #1286

merged 12 commits into from
Nov 13, 2023

Conversation

ramsessanchez
Copy link
Contributor

@ramsessanchez ramsessanchez commented Nov 7, 2023

Begin Work for Async -> Sync Revert
fixes microsoft/kiota-java#175
Needs: tests still need to be fixed

baywet
baywet previously approved these changes Nov 7, 2023
@ramsessanchez ramsessanchez marked this pull request as ready for review November 8, 2023 18:53
@ramsessanchez ramsessanchez requested a review from a team as a code owner November 8, 2023 18:53
@baywet
Copy link
Member

baywet commented Nov 9, 2023

@ramsessanchez: @andrueastman and I had a quick chat the other day. I think this is what we'll end up doing:

  • get rid of service exception, and replace it by api exception (already done)
  • throw directly the API exceptions, do not wrap them. Depending on whether we land on a checked or unchecked exception (kiota java discussion), this will need to be declared in methods prototype
  • for client exceptions, use native exceptions where it makes sense (like IllegalArgumentException, InterruptedException, etc...) and throw them.
  • if we still need a client exception, keep that class, do not inherit from Api exception, throw it without wrapping it, (declare in prototypes). But it should only be used for client behaviour issues (nothing related to the service side), and when it can't be replaced by native exceptions.

Hopefully this clarifies things, and I think you have a connect scheduled with Andrew today anyway.

Copy link
Member

@baywet baywet left a comment

Choose a reason for hiding this comment

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

Thanks for making the changes

@ramsessanchez
Copy link
Contributor Author

Once Kiota-Java is bumped to 0.9.0 this should be good to go

Copy link

sonarcloud bot commented Nov 13, 2023

SonarCloud Quality Gate failed.    Quality Gate failed

Bug B 1 Bug
Vulnerability A 0 Vulnerabilities
Security Hotspot A 0 Security Hotspots
Code Smell A 9 Code Smells

0.0% 0.0% Coverage
0.0% 0.0% Duplication

idea Catch issues before they fail your Quality Gate with our IDE extension sonarlint SonarLint

@ramsessanchez ramsessanchez merged commit 7903086 into v3/longTermBranch Nov 13, 2023
8 of 9 checks passed
@ramsessanchez ramsessanchez deleted the rsh/syncRevert_java branch November 13, 2023 20:55
@baywet
Copy link
Member

baywet commented Nov 13, 2023

@ramsessanchez can you have a look at the latest sonarcloud analysis please? There seems to be a mismatch between the method metadata and its behaviour (nullability)

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.

2 participants