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

Add PolicyDefinition.ExtensionData, Queue.ExtensionData, IMC.GetQueuesWithoutStatsAsync #285

Merged
merged 17 commits into from
Dec 21, 2023

Conversation

inikulshin
Copy link
Contributor

@inikulshin inikulshin commented Dec 14, 2023

Support custom policy definitions in GET and PUT.
Support unmapped queue stats.

Add IManagementClient.GetQueuesWithoutStatsAsync

http://localhost:15672/api/index.html

It is possible to disable the statistics in the GET requests and obtain just the basic information of every object. This reduces considerably the amount of data returned and the memory and resource consumption of each query in the system. For some monitoring and operation purposes, these queries are more appropiate. The query string parameter disable_stats set to true will achieve this.

I expect more comments this time :)
I don't commit approved.txt until when.

More changes can follow this PR, once approved.

  1. All extensions that receive Queue queue parameter can be changed to receive QueueName queueName.
  2. More records (everyone that's returned by GetAsync?) should also have [JsonExtensionData()]. But also in those that are used in PutAsync and PostAsync - to allow easy workaround for any missing or recently added property.
  3. Extensions should be organized... Maybe one file for Async extensions and another one for sync ones. Coverage should also be tested by GeneratePublicApi for ManagementClient and ManagementClientExtensions, some simple string replaces and equality assertion.

But let's discuss this one first.

@inikulshin
Copy link
Contributor Author

@Pliner @zidad please, review and comment.

@inikulshin
Copy link
Contributor Author

@Pliner @zidad @micdenny @luigiberrettini please, review and comment.

@zidad
Copy link
Member

zidad commented Dec 20, 2023

@inikulshin many thanks for this! What is the reason for adding the sync methods, do you really need them in your projects and why? Personally I would prefer to remove or obsolete all synchronous operations to simplify and clean the code base.

@inikulshin
Copy link
Contributor Author

inikulshin commented Dec 20, 2023

@inikulshin many thanks for this! What is the reason for adding the sync methods, do you really need them in your projects and why? Personally I would prefer to remove or obsolete all synchronous operations to simplify and clean the code base.

I don't really need those few sync extensions I added, but I think that sync extensions must be either for all async API or for none (as you suggested). Sync extensions still don't cover all async API because I think that it should be a different PR.

Personally I prefer to use those sync extensions and avoid await in almost every line.

P.S. Those extensions can be put in different extension package.

@zidad
Copy link
Member

zidad commented Dec 20, 2023

@inikulshin many thanks for this! What is the reason for adding the sync methods, do you really need them in your projects and why? Personally I would prefer to remove or obsolete all synchronous operations to simplify and clean the code base.

I don't really need those few sync extensions I added, but I think that sync extensions must be either for all async API or for none (as you suggested). Sync extensions still don't cover all async API because I think that it should be a different PR.

I agree... It's fine for now but I think there's almost no valid use cases anymore to use sync versions of methods in applications, at least I haven't encountered any in the recent years.

Personally I prefer to use those sync extensions and avoid await in almost every line.

It's unnecessarily verbose I agree but that just seems to be the standard way of calling pretty much every .net method nowadays :/

P.S. Those extensions can be put in different extension package.

But what would be a use case where this is necessary? Not sure if "I just don't like to write await" is a strong enough reason to create a separate package for this :)

zidad
zidad previously approved these changes Dec 20, 2023
@zidad zidad merged commit 490e21e into EasyNetQ:master Dec 21, 2023
5 checks passed
@inikulshin inikulshin deleted the queue branch December 24, 2023 11:31
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