Skip to content

Conversation

@artursouza
Copy link
Contributor

Description

Remove all global state in from setProperty
Added properties to ActorClient
Fixed constructor visibility for ActorClient

Issue reference

We strive to have all PR being opened based on an issue, where the problem or feature have been discussed prior to implementation.

Please reference the issue this PR will close: N/A

Checklist

Please make sure you've completed the relevant tasks for this PR, out of the following list:

  • Code compiles correctly
  • Created/updated tests
  • Extended the documentation

@artursouza artursouza force-pushed the stable_tests branch 4 times, most recently from 1d0cd03 to bb3c173 Compare October 1, 2024 07:17
Copy link
Contributor

@artur-ciocanu artur-ciocanu left a comment

Choose a reason for hiding this comment

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

@artursouza the PR looks great! I have left a few tiny comments regarding switching to use Map.of() instead of relying on Collections.unmodifiableMap().

@artursouza artursouza marked this pull request as ready for review October 1, 2024 07:46
@artursouza artursouza requested review from a team as code owners October 1, 2024 07:46
@artur-ciocanu
Copy link
Contributor

@artursouza you might want to re-trigger the build. It seems that there is race condition between Testcontainers and Spring Boot Test Context for a particular test.

Signed-off-by: Artur Souza <asouza.pro@gmail.com>
@artur-ciocanu
Copy link
Contributor

@cicoyle we need your approval again since the PR has changed. Thank you!

@artur-ciocanu
Copy link
Contributor

@artursouza it's not critical, but there a few more places in the PR where I mentioned that we could use Map.of().

@artursouza artursouza force-pushed the stable_tests branch 2 times, most recently from 20bf045 to e4a373c Compare October 2, 2024 22:05
Signed-off-by: Artur Souza <asouza.pro@gmail.com>
@artursouza artursouza changed the title Remove all global state in from setProperty Remove all global state from setProperty Oct 2, 2024
}

@Test
@Disabled("Test is flaky due to global state in the spring test application.")
Copy link
Contributor Author

Choose a reason for hiding this comment

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

@salaboy This test is flaky, it has global state for setting properties somewhere. I disabled it until we find it. Can you help in a new PR after this?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Created a tracking issue.

@artursouza
Copy link
Contributor Author

@artursouza it's not critical, but there a few more places in the PR where I mentioned that we could use Map.of().

Done

cicoyle
cicoyle previously approved these changes Oct 2, 2024
Copy link
Contributor

@cicoyle cicoyle left a comment

Choose a reason for hiding this comment

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

lgtm

cicoyle
cicoyle previously approved these changes Oct 2, 2024
Signed-off-by: Artur Souza <asouza.pro@gmail.com>
@artursouza artursouza merged commit 7490434 into dapr:master Oct 2, 2024
6 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