Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Do not overwrite existing IConfiguration #16

Closed
wants to merge 1 commit into from
Closed

Do not overwrite existing IConfiguration #16

wants to merge 1 commit into from

Conversation

waynebrantley
Copy link
Contributor

host.json is read and used by the runtime. Extension settings are applied at startup for things like queue times, etc.

  "extensions": {
    "queues": {
      "maxPollingInterval": "00:00:02",
      "visibilityTimeout": "00:00:30",
      "batchSize": 16,
      "maxDequeueCount": 5,
      "newBatchThreshold": 8
    }
  } 

However, when your code overwrites the IConfiguration all these settings get removed.
This PR leverages an overload designed to alter the configuration by adding app settings.
Additionally, there are new extensions to assist when needing the environment name (like when setting up logging, etc).
Using context, was able to make useLogger not do a manual lookup in IServiceCollection

Once these are in place, you still get all the features you were intending on adding plus all the built in ones.

@junalmeida
Copy link
Owner

@waynebrantley
I loved your changes, I will try to get some time to make a test this weekend.
I have one question for you, is your assumption based on the very latest version of the library? I'm asking because it indeed removed all previous host.json configs in previous version, but I've made changes to avoid that on the very latest version. If you are using the latest version it means that didn't work for you, right?
In any case your code seems better, if it works I will merge.

@waynebrantley
Copy link
Contributor Author

I had not updated to your very latest version. I looked at your changes and it did not seem that it would help with this situation. Thanks for considering!

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