ThreeMammals
Added AddOcelot Configbuilder Overload
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.
New Feature #
Proposed Changes
- Added a new overload to the AddOcelot in ConfigurationBuilder, to remove ocelot file dependency from the application.
@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?
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!
@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!
@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? 😉
@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.
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.
@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...
@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.
@vijay-karavadra commented on Sep 8
Yes, for configuration extensions we do have unit tests only. Thanks for covering your method by new unit tests and updating others!
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!
@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.