Ocelot icon indicating copy to clipboard operation
Ocelot copied to clipboard

#651 Merge custom properties of config file

Open jlukawska opened this issue 4 years ago • 6 comments

Closes #651

  • #651

Proposed Changes

  • The whole sections of config files (with custom properties) are merged.

jlukawska avatar Apr 07 '20 19:04 jlukawska

Congrats, @jlukawska ! 🎉 Your feature branch has been rebased onto ThreeMammals:develop.

We can go with code review now...

raman-m avatar Jul 26 '23 15:07 raman-m

@nurhat As the #651 issue author, could you review this solution please? Will the solution be useful and helpful for your daily gateway development?

raman-m avatar Aug 03 '23 18:08 raman-m

@RaynaldM @wast You are great approvers! 🥇 Seems like @jlukawska is making breaking changes and you've approved them. 🤣 I am not sure about changes that they're breaking on 100%, but I will review the code thoroughly. 👇

raman-m avatar Aug 03 '23 18:08 raman-m

@nurhat @jlukawska I would like to understand, how useful this feature for real world user scenarios.

User case 1

Change behavior of PollyQoSProvider

Given, I wish to override behavior of the Quality of Service feature by changes in PollyQoSProvider and/or AddPolly and/or PollyCircuitBreakingDelegatingHandler classes. And, I will define custom property of the route for custom QoS overriding in a ocelot.xservices.json file. And, I will read the custom property from DownstreamRoute object after merging all configs. When, I read the custom property from DownstreamRoute object. Then, I get stuck! 😉

Question:

  • How to read custom property from DownstreamRoute object?
  • How and what interface do I need to inject to a service class to be able to read custom property from DownstreamRoute object?

raman-m avatar Aug 03 '23 19:08 raman-m

@raman-m, I marked this PR as a draft, because I am not sure if the conflicts are resolved correctly (even though the tests passed correctly). It wasn't that easy ;) I'd have to revise the code, understand the changes from develop branch and maybe the PR code would have to be improved. Apart from that, would this PR be helpful for anyone? I proposed it 4 years ago and the issue reporter no longer responds.

jlukawska avatar May 16 '24 17:05 jlukawska

Understood, Jolanta. It seems you're uncertain and wish to further develop. I also notice that many PR files contain a fake diff indicating numerous merge conflicts were resolved.

Here's what I suggest:

  • Create a new feature branch from develop. Your forked repo develop is synced.
  • Clone the second repository for manual file operations.
  • Transfer all changes from the old feature branch to the new one using a file manager (Total Commander is my choice, an old school tool with a dual-pane interface vs useful comparison command).
  • Open a new PR after confirming successful compilation and passing tests.
  • Close this old PR and reference the new one.

Does this sound like a good approach?

P.S. I believe the most important changes are in the file: src/Ocelot/DependencyInjection/ConfigurationBuilderExtensions.cs which should be double checked.

raman-m avatar May 28 '24 09:05 raman-m