Skip to content

Conversation

@plyhun
Copy link
Contributor

@plyhun plyhun commented Dec 27, 2023

Abstract

One should never store a thing in ThreadLocal, since we operate async contexts, not threads.

Checklist

General

  • Added abstract that describes the change
  • Added changelog entry to /CHANGELOG.adoc
  • Ensured that the change is covered by tests
  • Ensured that the change is documented in the docs

On API Changes

  • Checked if the changes are breaking or not
  • Added GraphQL API if applicable
  • Added Elasticsearch mapping if applicable

@plyhun plyhun requested a review from npomaroli December 29, 2023 10:26
Tx.threadLocalGraph.set(tx);
Optional.ofNullable(Vertx.currentContext())
.ifPresentOrElse(
ctx -> ctx.putLocal("tx", Optional.ofNullable(tx)),
Copy link
Contributor Author

Choose a reason for hiding this comment

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

The actual change. There are get() and getLocal() in the Vert.x Context, which seem to be functionally equal, but getLocal() has internal usages.

return Tx.threadLocalGraph.get();
Optional<Context> maybeVertxContext = Optional.ofNullable(Vertx.currentContext());
if (maybeVertxContext.isPresent()) {
Context vertxContext = maybeVertxContext.get();
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Don't mix the cases where we have an empty Context, and where we have no Context at all.

plyhun added 5 commits January 3, 2024 09:23
This reverts commit 2ed9611.
This reverts commit 13195ff.
This reverts commit 0cec36b.
This reverts commit 16a1700.
@plyhun
Copy link
Contributor Author

plyhun commented Jan 3, 2024

Postponed by now.

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