Skip to content

Fix blocking call when deleting previous challenges#54

Merged
bmd007 merged 1 commit into
bmd007:mainfrom
Arooba-git:fix-blocking-in-WebfluxAllFluxCaller
Aug 21, 2023
Merged

Fix blocking call when deleting previous challenges#54
bmd007 merged 1 commit into
bmd007:mainfrom
Arooba-git:fix-blocking-in-WebfluxAllFluxCaller

Conversation

@Arooba-git

Copy link
Copy Markdown
Contributor

Hello :)
Thank you for this handy project!

This PR fixes a blocking call and consequently a failing test case in WebFluxClient tests:
Screen Shot 2023-07-28 at 2 49 44 PM

Screen Shot 2023-07-28 at 2 56 46 PM

@bmd007

bmd007 commented Aug 21, 2023

Copy link
Copy Markdown
Owner

Hello :) Thank you for this handy project!

This PR fixes a blocking call and consequently a failing test case in WebFluxClient tests: Screen Shot 2023-07-28 at 2 49 44 PM

Screen Shot 2023-07-28 at 2 56 46 PM

thanks very much for kind words and effort to contribute. Here, IMO, usage of delayUntil to wrap the deleteAllPreviousChallenges seems more appropriate. Mono.fromRunnable is a change in a place were there is no problem.

btw, you seem like an AI agent?! Did I get it right?

@bmd007 bmd007 merged commit 698967e into bmd007:main Aug 21, 2023
bmd007 added a commit that referenced this pull request Aug 21, 2023
bmd007 added a commit that referenced this pull request Aug 21, 2023
@bmd007

bmd007 commented Aug 21, 2023

Copy link
Copy Markdown
Owner

I accidentally merged your PR. Then I merged the revert of it. Please feel free to create a new PR.

@Arooba-git

Copy link
Copy Markdown
Contributor Author

btw, you seem like an AI agent?! Did I get it right?

Haha no I'm a human 😅 . Actually as part of our research we are going through projects containing blocking calls and submitting fix PRs, to gauge developers responses. As we had a tight schedule , some PRs have some portions of text copy pasted (as our approach remains the same), but the detected bugs, of course, are unique to projects :)

@bmd007

Mono.fromRunnable is a change in a place were there is no problem.

Could you kindly elaborate a bit more on this? 🤔

@bmd007

bmd007 commented Aug 21, 2023

Copy link
Copy Markdown
Owner

Great response, thanks.

I reckoned something about fixing blocking calls is going on.

The intention with .block here is to make sure previous challenges are deleted before the actual test code runs. (Clean up, so to say).

To solve that non blocking ly we can connect it to the main test code stream by wrapping it with delay until.

The issue with your approach is that I'm not sure if the runnable is using all the reactive magic. Maybe it does.

@Arooba-git

Arooba-git commented Aug 21, 2023

Copy link
Copy Markdown
Contributor Author

the runnable is using all the reactive magic

Oh.. honestly I'm not pro at reactive programming but AFAIK, the then operator is simply used to wait for the previous operation to complete to execute the mono passed to it right? so I still don't get how the mono.fromRunnable wrapper is disrupting the reactiveness of pipeline 🤔

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