Refactor api client to adhere to rate limits

[danischm]

This PR introduces the following changes/enhancements, while trying to retain the current structure and implementation as much as possible:

• Remove unused elements in api_client.go like apiURL, FileDownload, SaveDownload
• Update comments referring to DNAC instead of Meraki
• Remove NewClientNoAuth() and NewClientWithOptionsNoAuth() as the client does not authenticate actively anyway and the current implementation can be confusing, as you would end up with a client without a User-Agent string defined and no retries enabled
• Set the min wait time for retries to 1s and the max wait time to 5s using the default resty retry mechanism with exponential backoff
• Retry not only on 429 responses but also on request errors to account for intermittent network issues
• Respect Retry-After header in responses
• Implement rate limit bucket to adhere to API rate limit of 10 requests per second, currently takes a conservative/simplified approach of limiting all API requests to 10 per second which could be further improved in the future to also take into account the organization and limit it to 10 requests per second per organization
• Introduce generic Get/Post/Put/Delete functions to reduce the boilerplate code in service implementations
• Introduce tests covering the api_client.go code
• Update administered.go service to use newly introduced Get function

TODO: Update remaining services to use new Get/Post/Put/Delete functions using existing code generation tools.

[fmunozmiranda]

Remove NewClientNoAuth() and NewClientWithOptionsNoAuth() as the client does not authenticate actively anyway and the current implementation can be confusing, as you would end up with a client without a User-Agent string defined and no retries enabled

This point was not considered, since it has its function to create a client from environment variables.

[fmunozmiranda]

• Remove NewClientNoAuth() and NewClientWithOptionsNoAuth() as the client does not authenticate actively anyway and the current implementation can be confusing, as you would end up with a client without a User-Agent string defined and no retries enabled

Please don´t remove it. It is necesary for client creation with env vars.

If you can put ratelimiter.Wait(1) directly in function, and not use new POST, GET, DELETE. PUT methods it would be better for us, because in that way it doesn´t represents a big change in or generation tool.

[danischm]

Please don´t remove it. It is necesary for client creation with env vars.

Where is it being used? Neither NewClientNoAuth() nor NewClientWithOptionsNoAuth() is being used in the provider. If you would use them you would end up with a client that does not have a User-Agent string defined and also the resty retry mechanism is not configured which makes the client behave differently than the one created by NewClient() or NewClientWithOptions() functions.

[danischm]

If you can put ratelimiter.Wait(1) directly in function, and not use new POST, GET, DELETE. PUT methods it would be better for us, because in that way it doesn't represents a big change in or generation tool.

The reason I created this functions is to have a place to later introduce more granular rate limiting controls as the server side rate limiting is done per organization and on the other hand it reduces the boilerplate code in the services implementations. It should actually simplify the code generation as you would only need to replace the result and body data structure names in a single line of code.

[fmunozmiranda]

These are used internally by the api_client, to avoid confusion, in the readme, you have the way in which the provider is used.

https://github.com/meraki/dashboard-api-go/tree/develop?tab=readme-ov-file#parameters

[danischm]

These are used internally by the api_client, to avoid confusion, in the readme, you have the way in which the provider is used.

Both these functions shown in the readme (NewClient() or NewClientWithOptions()) are still there and work as before. I did not remove them.

[fmunozmiranda]

If you can put ratelimiter.Wait(1) directly in function, and not use new POST, GET, DELETE. PUT methods it would be better for us, because in that way it doesn't represents a big change in or generation tool.

The reason I created this functions is to have a place to later introduce more granular rate limiting controls as the server side rate limiting is done per organization and on the other hand it reduces the boilerplate code in the services implementations. It should actually simplify the code generation as you would only need to replace the result and body data structure names in a single line of code.

Yes, I understand what are you saying, but for us is better to have it separated, in case we have to custom one of them, please, help us with that.

[fmunozmiranda]

These are used internally by the api_client, to avoid confusion, in the readme, you have the way in which the provider is used.

Both these functions shown in the readme (NewClient() or NewClientWithOptions()) are still there and work as before. I did not remove them.

Ok, I think we can allow it. Thank you

[danischm]

Yes, I understand what are you saying, but for us is better to have it separated, in case we have to custom one of them, please, help us with that.

I can update the code as you said, but I would suggest to still keep the generic Get/Post/Put/Delete functions as they could be useful also outside the current services implementation. Would you agree?

[fmunozmiranda]

Yes, I understand what are you saying, but for us is better to have it separated, in case we have to custom one of them, please, help us with that.

I can update the code as you said, but I would suggest to still keep the generic Get/Post/Put/Delete functions as they could be useful also outside the current services implementation. Would you agree?

Ok.

[fmunozmiranda]

If you can set as configurable, the requestPerSecond client can do, would be perfect too

[danischm]

If you can set as configurable, the requestPerSecond client can do, would be perfect too

But just via environment variable or also include it as a mandatory parameter in the NewClientWithOptions() function?

[fmunozmiranda]

Both, I think is better

[fmunozmiranda]

You can have it like a default of 10 if user don´t specify

[danischm]

Ok. If we change NewClientWithOptions() to something like this:

func NewClientWithOptions(baseURL string, dashboardApiKey string, debug string, sslVerify string, requestsPerSecond int)

Which is not ideal as the user would need to know what the appropriate value is and we could not really make use of a default value. Another option is to introduce a separate function:

func NewClientWithOptionsAndRequests(baseURL string, dashboardApiKey string, debug string, sslVerify string, requestsPerSecond int)

Or make use of the Functional Optionals Pattern.

[fmunozmiranda]

I think if we have this:

func NewClientWithOptionsAndRequests(baseURL string, dashboardApiKey string, debug string, sslVerify string, requestsPerSecond int)

And something like:

client.SetRequestPerSecond(i int)

Would be perfect-