ThreeMammals / Ocelot

.NET API Gateway
https://www.nuget.org/packages/Ocelot
MIT License
8.38k stars 1.64k forks source link

#1700 Create custom Ocelot config file when instantiating steps during parallel execution #1703

Closed ggnaegi closed 1 year ago

ggnaegi commented 1 year ago

Fixes #1700

When instantiating steps, creating a custom ocelot config file name. Finally deleting the newly created config file when disposing the Step instance.

Proposed Changes

raman-m commented 1 year ago

Wow! Cool! Let's make 10 PR builds of 10 commits being added to this PR, to get some statistics on stability of this fix.

Just commit a small change of Ocelot core, not tests, and the 2nd commit should be revert commit of previous one. So, there will be 5 commits and 5 revert-commits.

Good plan?

ggnaegi commented 1 year ago

@raman-m no way, that's really funny ;-) 6 out of 10... I can't reproduce that on my machine

ggnaegi commented 1 year ago

@raman-m Ok, a bit better now, 9 out of 11...

ggnaegi commented 1 year ago

@raman-m why did you remove xunit.runner.json?

raman-m commented 1 year ago

I've reverted commit 9fac778 Sorry!

It seems your solution doesn't work for some reason... That's strange... It should...

Have a rest today! 😉 We will continue tomorrow, this week...

raman-m commented 1 year ago

@ggnaegi commented

why did you remove xunit.runner.json?

Because you've disabled parallel execution for all tests by default.

ggnaegi commented 1 year ago

@ggnaegi commented

why did you remove xunit.runner.json?

Because you've disabled parallel execution for all tests by default.

Yeah, I tried...

ggnaegi commented 1 year ago

@raman-m 11 out of 12. It's not that bad...

ggnaegi commented 1 year ago

@raman-m we might have a solution...

raman-m commented 1 year ago

It seems this fix works! 👍 What was the root cause of failing? I don't think it was random port finder... 🤣

raman-m commented 1 year ago

Do you confirm that we can still use parallelization of all tests?

ggnaegi commented 1 year ago

Do you confirm that we can still use parallelization of all tests?

@raman-m Yes, we still have an issue with Open_circuit_should_not_effect_different_route though... That's the test failing at the minute. I can reproduce that on my computer, it fails after 20 minutes.

raman-m commented 1 year ago

Open_circuit_should_not_effect_different_route

Forget about this test now. It is out of the scope of linked bug-issue. 🆗 I'm going to review...

ggnaegi commented 1 year ago

Open_circuit_should_not_effect_different_route

Forget about this test now. It is out of the scope of linked bug-issue. 🆗 I'm going to revi

It seems this fix works! 👍 What was the root cause of failing? I don't think it was random port finder... 🤣

@raman-m No it wasn't, but the random port finder code doesn't seem to be thread safe, since the concurrentbag would allow identical values to be stored and the method can be accessed by several threads concurrently

ggnaegi commented 1 year ago

@raman-m imho ok to merge

raman-m commented 1 year ago

Revert "commit #h5" 6e4b8a4

@ggnaegi #1706 FYI

raman-m commented 1 year ago

@ggnaegi Congrats! We did that! Thank you very much for your efforts during this Sunday's evening!