#1228 #1235 #1247 Overloaded AddOcelot method for dynamic FileConfiguration construction #1569

vijay-karavadra · 2022-03-26T06:33:45Z

Closes #1228 #1235 #1247

New Feature

Added an AddOcelot overload to load FileConfiguration directly from the application, so that all the routes could be made configurable and could be load from anywhere.

Proposed Changes

Added a new overload to the AddOcelot in ConfigurationBuilder to remove Ocelot file dependency from the application.

vijay-karavadra · 2022-03-26T06:44:11Z

@TomPallister Please review.

raman-m · 2023-05-13T11:15:55Z

@vijay-karavadra Hi Vijay!
What issue is this PR related to?

Could you write a couple of unit/integration tests for your proposed feature, please?

raman-m

Please, fix minor issues!

Also, I would welcome you to write unit and/or integration test(s).

src/Ocelot/DependencyInjection/ConfigurationBuilderExtensions.cs

Added an AddOcelot overload to load FileConfiguration directly from the application, so that all the routes could be made configurable and could be load from anywhere.

vijay-karavadra · 2023-07-10T18:09:46Z

Hey @raman-m ,

First of all, thanks for the review. I appreciate it.

I had added this overload because I wanted to load ocelot configuration from a third party configuration manager(AWS SSM). So, I was thinking to load them on startup and then seed them into ocelot with this overload.

So, the thing is I'm a little bit busy right now and this has been deprioritized at the moment. But, I still think that this feature makes sense, so I'll try to give some time to it by this or next weekend and will try to complete it.

Thanks!

raman-m · 2023-07-11T10:16:32Z

@vijay-karavadra commented on Jul 10, 2023

I had added this overload because I wanted to load ocelot configuration from a third party configuration manager(AWS SSM). So, I was thinking to load them on startup and then seed them into ocelot with this overload.

OK. New extensions for the IConfigurationBuilder interface are welcome! We created the similar extensions for IMvcCoreBuilder interface, see #1655 please. You could get some inspiration from this PR.

But, I still think that this feature makes sense, so I'll try to give some time to it by this or next weekend and will try to complete it.

Sure thing, Vijay!

Pay attention that if you introduce new extension-method, it is required to update docs to show how to use your extended functionality. More in #1655 .
Ping me if you need a help!

raman-m · 2023-07-11T10:22:52Z

Is this PR related to some existed issue?

raman-m · 2023-09-07T14:43:54Z

@vijay-karavadra Hi!
How was your summer?
Are you going to contribute (fix code review issues, write unit tests)?

Pay attention this is production ready project. A number of projects use this gateway.
And the quality of the source code should be high.
For new functionality unit & acceptance tests must be written. But especially for this PR only unit test could be accepted.
It is better to update docs too. Tom can ask to write examples, code snippets in docs.

Are you ready for new code challenges? 😉

raman-m · 2023-09-07T14:51:36Z

@vijay-karavadra commented on Jul 10:

I had added this overload because I wanted to load ocelot configuration from a third party configuration manager (AWS SSM). So, I was thinking to load them on startup and then seed them into ocelot with this overload.

Do you have a successful and workable solution now? I meant store file config object in AWS SSM, inject this object during app startup, pass the object to your extension-method...

It seems we have to write some acceptance test(s) to emulate AWS environment, and integrate to Ocelot environment.
But having such solutions from your side, writing an acceptance tests won't be a difficult challenge.

vijay-karavadra · 2023-09-08T06:51:53Z

Hey @raman-m ,

I had a great summer! I hope you had a great one too and also you are doing well.

I was somewhat busy in some projects, thanks for bringing me back to this. I appreciate your patience and I grasp the gravity and significance of this project.

I added the code review changes as per suggestions, please review. Also I started working on unit and acceptance tests and will push once done.

raman-m · 2023-09-08T09:06:53Z

@vijay-karavadra commented on Sep 8

Perfect! Added code is fine. 2 code review issues have been resolved.
Waiting for the next commits with tests...

vijay-karavadra · 2023-09-08T10:46:37Z

@raman-m

I have recently incorporated unit tests, and I would greatly appreciate your review of them. In regard to the acceptance tests, I've conducted a thorough examination of the existing ones. It appears that the acceptance tests have been designed under the presumption that the configuration builder functions seamlessly. To clarify, the configuration builder is not included as a component of the acceptance tests. Nevertheless, I am open to your insights and suggestions, so please do inform me if there are any specific tests you believe should be integrated into our suite of unit and acceptance tests.

…inalize(object). This will prevent derived types that introduce a finalizer from needing to re-implement 'IDisposable' to call it.

raman-m

One more approval is required.

vijay-karavadra · 2023-09-23T15:01:37Z

Thanks @raman-m . It was a very good experience in working in this library. Feel free to assign me any issues in this library where the help is needed. I'd be happy to help!

raman-m · 2023-10-09T14:55:59Z

@vijay-karavadra commented on Sep 23

You are welcome!
Much help is needed in backlog. There are a lot of issues! And you are free to assign any of them 😉
I would suggest to assign a bug to yourself, with Accepted label or not, because bugs have highest priority. Skip bugs which have attached PR, for sure. So, take any bug without a PR. And ping me a message in the bug's thread, and you'll be assigned.

raman-m assigned vijay-karavadra May 13, 2023

raman-m force-pushed the patch-1 branch from b9ff360 to 563aa04 Compare May 13, 2023 11:08

raman-m self-requested a review May 13, 2023 11:13

raman-m added feature A new feature small effort Likely less than a day of development effort. proposal Proposal for a new functionality in Ocelot waiting Waiting for answer to question or feedback from issue raiser labels May 13, 2023

raman-m requested changes May 13, 2023

View reviewed changes

src/Ocelot/DependencyInjection/ConfigurationBuilderExtensions.cs Outdated Show resolved Hide resolved

src/Ocelot/DependencyInjection/ConfigurationBuilderExtensions.cs Outdated Show resolved Hide resolved

raman-m changed the title ~~Added AddOcelot Configbuilder Overload~~ Added overloaded AddOcelot method as an extension of the IConfigurationBuilder interface May 13, 2023

Added AddOcelot Configbuilder Overload

b888c74

Added an AddOcelot overload to load FileConfiguration directly from the application, so that all the routes could be made configurable and could be load from anywhere.

raman-m force-pushed the patch-1 branch from 563aa04 to b888c74 Compare July 10, 2023 18:00

raman-m added medium effort Likely a few days of development effort and removed small effort Likely less than a day of development effort. proposal Proposal for a new functionality in Ocelot labels Jul 11, 2023

raman-m added the question Initially seen a question could become a new feature or bug or closed ;) label Sep 7, 2023

Code review changes

9e061a0

Added unit tests for ConfigurationBuilder AddOcelot method overload

005d5ff

vijay-karavadra requested a review from raman-m September 8, 2023 10:55

raman-m added 2 commits September 13, 2023 12:45

code review

7e78af9

CA1816 Change ConfigurationReloadTests.Dispose() to call GC.SuppressF…

e75cdcc

…inalize(object). This will prevent derived types that introduce a finalizer from needing to re-implement 'IDisposable' to call it.

raman-m requested review from binarymash, jps, geffzhang, RaynaldM, philproctor, anktsrkr, sharpn, abooraja, wast, yangzhongke and thiagoloureiro September 13, 2023 15:54

raman-m approved these changes Sep 13, 2023

View reviewed changes

RaynaldM approved these changes Sep 18, 2023

View reviewed changes

Merge branch 'develop' into patch-1

afb8aeb

raman-m requested a review from TomPallister September 20, 2023 12:15

raman-m added accepted Bug or feature would be accepted as a PR or is being worked on and removed needs feedback Issue is waiting on feedback before acceptance labels Sep 20, 2023

RaynaldM approved these changes Sep 21, 2023

View reviewed changes

raman-m linked an issue Sep 21, 2023 that may be closed by this pull request

Extend AddOcelot to EnumerateFiles in subfdirectories #1235

Closed

raman-m mentioned this pull request Sep 21, 2023

Extend AddOcelot to EnumerateFiles in subfdirectories #1235

Closed

This was linked to issues Sep 21, 2023

Upgrade to 3.1 produces empty ocelot.json #1247

Closed

Dynamically set/add Configuration for rate limiting #1228

Closed

raman-m mentioned this pull request Sep 21, 2023

Reload merged configuration files #872

Closed

raman-m changed the title ~~Added overloaded AddOcelot method as an extension of the IConfigurationBuilder interface~~ #1228 #1235 #1247 Overloaded AddOcelot method for dynamic FileConfiguration construction Sep 21, 2023

Merge branch 'develop' into patch-1

737ed28

raman-m merged commit 4d245ec into ThreeMammals:develop Sep 23, 2023
2 checks passed

raman-m mentioned this pull request Oct 3, 2023

Follow up #1670: Fix StyleCop issues and improve the code #1711

Merged

vijay-karavadra deleted the patch-1 branch October 10, 2023 09:50

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

#1228 #1235 #1247 Overloaded AddOcelot method for dynamic FileConfiguration construction #1569

#1228 #1235 #1247 Overloaded AddOcelot method for dynamic FileConfiguration construction #1569

vijay-karavadra commented Mar 26, 2022 •

edited by raman-m

vijay-karavadra commented Mar 26, 2022

raman-m commented May 13, 2023

raman-m left a comment

vijay-karavadra commented Jul 10, 2023

raman-m commented Jul 11, 2023

raman-m commented Jul 11, 2023

raman-m commented Sep 7, 2023

raman-m commented Sep 7, 2023

vijay-karavadra commented Sep 8, 2023

raman-m commented Sep 8, 2023 •

edited

vijay-karavadra commented Sep 8, 2023

raman-m left a comment

vijay-karavadra commented Sep 23, 2023

raman-m commented Oct 9, 2023 •

edited

#1228 #1235 #1247 Overloaded AddOcelot method for dynamic FileConfiguration construction #1569

#1228 #1235 #1247 Overloaded AddOcelot method for dynamic FileConfiguration construction #1569

Conversation

vijay-karavadra commented Mar 26, 2022 • edited by raman-m

Closes #1228 #1235 #1247

New Feature

Proposed Changes

vijay-karavadra commented Mar 26, 2022

raman-m commented May 13, 2023

raman-m left a comment

Choose a reason for hiding this comment

vijay-karavadra commented Jul 10, 2023

raman-m commented Jul 11, 2023

raman-m commented Jul 11, 2023

raman-m commented Sep 7, 2023

raman-m commented Sep 7, 2023

vijay-karavadra commented Sep 8, 2023

raman-m commented Sep 8, 2023 • edited

vijay-karavadra commented Sep 8, 2023

raman-m left a comment

Choose a reason for hiding this comment

vijay-karavadra commented Sep 23, 2023

raman-m commented Oct 9, 2023 • edited

vijay-karavadra commented Mar 26, 2022 •

edited by raman-m

raman-m commented Sep 8, 2023 •

edited

raman-m commented Oct 9, 2023 •

edited