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

doc: @Blocking #7228

Merged
merged 7 commits into from Apr 29, 2022
Merged

doc: @Blocking #7228

merged 7 commits into from Apr 29, 2022

Conversation

wetted
Copy link
Member

@wetted wetted commented Apr 18, 2022

Closes #7183

@wetted wetted requested review from mattmoss and sdelamo April 18, 2022 18:26
@jameskleeh
Copy link
Contributor

jameskleeh commented Apr 19, 2022

I don't think we should document @Blocking as it relates to controller routes because @ExecuteOn should be used instead. That use case should be deprecated and removed in a future version imo. The same goes for thread-selection: AUTO

@wetted
Copy link
Member Author

wetted commented Apr 19, 2022

I don't think we should document @Blocking as it relates to controller routes because @ExecuteOn should be used instead. That use case should be deprecated and removed in a future version imo. The same goes for thread-selection: AUTO

I’m OK with that either way. I already have a PR that removes the other [obsolete] mention of it in core docs.

If we want to deprecate its use and not mention in in the user guide, should we meta-annotate it @Internal for now?

@sdelamo
Copy link
Collaborator

sdelamo commented Apr 19, 2022

I think we should document it. I don't think we should not document a feature just because we plan to remove it.

@sdelamo sdelamo added this to the 3.4.3 milestone Apr 19, 2022
@sdelamo sdelamo removed the request for review from mattmoss April 19, 2022 11:16
@sdelamo
Copy link
Collaborator

sdelamo commented Apr 21, 2022

@wetted I have changed the copy. Let me know if you are ok with the changes.

@sdelamo sdelamo requested review from graemerocher and removed request for sdelamo April 21, 2022 08:18
@sdelamo sdelamo force-pushed the Issue7183-document-blocking branch from ee39dc1 to 2c2f3d0 Compare April 21, 2022 08:19
@sdelamo sdelamo changed the title Issue 7183 - document @Blocking annotation. doc: @Blocking Apr 21, 2022
@wetted
Copy link
Member Author

wetted commented Apr 21, 2022

@sdelamo yes, thanks for improving/correcting/clarifying. Looks good now.

@sdelamo
Copy link
Collaborator

sdelamo commented Apr 28, 2022

@graemerocher please review it when you can.


If you set `micronaut.server.thread-selection` to `AUTO`, The Micronaut Framework offloads the execution of methods annotated with `@Blocking` to the IO thread pool (See: api:io.micronaut.scheduling.TaskExecutors[]).

NOTE: `@Blocking` only works if you are using `AUTO` thread selection. Micronaut Framework defaults to `MANUAL` thread selection since Micronaut 2.0. We recommend the usage of ann:scheduling.annotation.ExecuteOn[] annotation to execute the blocking operations on a different thread. `@ExecutesOn` works for both `MANUAL` or `AUTO` thread selection.
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
NOTE: `@Blocking` only works if you are using `AUTO` thread selection. Micronaut Framework defaults to `MANUAL` thread selection since Micronaut 2.0. We recommend the usage of ann:scheduling.annotation.ExecuteOn[] annotation to execute the blocking operations on a different thread. `@ExecutesOn` works for both `MANUAL` or `AUTO` thread selection.
NOTE: `@Blocking` only works if you are using `AUTO` thread selection. Micronaut defaults to `MANUAL` thread selection since Micronaut 2.0. We recommend the usage of ann:scheduling.annotation.ExecuteOn[] annotation to execute the blocking operations on a different thread pool. `@ExecutesOn` works for both `MANUAL` and `AUTO` thread selection.

Copy link
Collaborator

Choose a reason for hiding this comment

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

Micronaut should not be used as a noun to match the brand guidelines. Hence, the Micronaut Framework

Copy link
Collaborator

Choose a reason for hiding this comment

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

I have incorporated most of your suggestions here: e33ddf1

Copy link
Member

Choose a reason for hiding this comment

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

There are gazillion places where simply "Micronaut" is used. Enforcing "Micronaut Framework" in every occurrence in the documentation is quite inappropriate, to say the least.

sdelamo and others added 2 commits April 29, 2022 10:53
…/atBlocking.adoc

Co-authored-by: Álvaro Sánchez-Mariscal <alvaro.sanchez-mariscal@oracle.com>
sdelamo and others added 2 commits April 29, 2022 11:22
…/atBlocking.adoc

Co-authored-by: Álvaro Sánchez-Mariscal <alvaro.sanchez-mariscal@oracle.com>
…/atBlocking.adoc

Co-authored-by: Álvaro Sánchez-Mariscal <alvaro.sanchez-mariscal@oracle.com>
@CLAassistant
Copy link

CLAassistant commented Apr 29, 2022

CLA assistant check
All committers have signed the CLA.

@sonarcloud
Copy link

sonarcloud bot commented Apr 29, 2022

Kudos, SonarCloud Quality Gate passed!    Quality Gate passed

Bug A 0 Bugs
Vulnerability A 0 Vulnerabilities
Security Hotspot A 0 Security Hotspots
Code Smell A 0 Code Smells

No Coverage information No Coverage information
No Duplication information No Duplication information

@sdelamo sdelamo merged commit ae292ba into 3.4.x Apr 29, 2022
@sdelamo sdelamo deleted the Issue7183-document-blocking branch April 29, 2022 09:50
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

document @Blocking
5 participants