Closed rajlearner17 closed 1 year ago
Questions around environment_type
and other config arguments -
environment_type
? Should it just be environment
or base_url
? In my opinion, this should not be a required parameter, rather it should default to production. https://registry.terraform.io/providers/n3integration/godaddy/latest/docs. Please confirm what the API actually defaults to.secret_key
argument should be renamed to api_secret
as per the sample mentioned below -
Calling the API
Here is a sample call using the API to check if a domain is available. Replace API_KEY and API_SECRETwith your API Key and Secret.
curl -X GET -H "Authorization: sso-key [API_KEY]:[API_SECRET]" "https://api.godaddy.com/v1/domains/available?domain=example.guru"
What is the reason behind naming it as environment_type? Should it just be environment or base_url? In my opinion, this should not be a required parameter, rather it should default to production. https://registry.terraform.io/providers/n3integration/godaddy/latest/docs. Please confirm what the API actually defaults to.
The GoDaddy SDK which is being used here, it is passing the base URL internally as per the type of GoDaddy environment we have weather we have a Production/Development environment.
I feel that the
secret_key
argument should be renamed toapi_secret
as per the sample mentioned below
Updated the argument secret_key
to api_secret
@ParthaI please take a look at the review comments. Thanks!!
Additional questions:
- Is there no consistency in the not found error codes? Does the API return a 404 error code? I think the if we just focus on the error code, we could wrap all the errors around
NOT_FOUND
.- Has the pagination been handled correctly? Judging by the API output, I think you could use https://github.com/turbot/steampipe-plugin-launchdarkly/blob/main/launchdarkly/table_launchdarkly_project.go#L134-L154 for reference
- Could you please attach a doc where the max limits of the API calls have been defined?
- Does the API doc mention anything about the rate limit errors?
There is no consistency in the not found error codes. The API does not return a 404 error code. It returns the error like Error: Order not found
, Error: NOT_FOUND
. We have handled the not found error codes as pare the error thrown by API here.
Performing the pagination operation on the APIs defers from SDK to SDK, I had tried couple of approaches but the approach which one we have currently looks better than other with minimum codes, I have tested it out, the code seems to be working fine for me. Do you want to change this code as per https://github.com/turbot/steampipe-plugin-launchdarkly/blob/main/launchdarkly/table_launchdarkly_project.go#L134-L154?
The max limit value is no where mentioned in the doc, as per the testing we have provided the value of 1000
to it. We have also tested the APIs by passing the max limit value of 10000
, the API doesn't throw any error.
Each API endpoint has an allotted number of calls you are allowed to make within a period of time (60 requests per minute). If you attempt to make more API calls to an endpoint than this allotted amount, the call will be rejected by the GoDaddy API and will return an HTTP status code of 429 and an error message indicating that your rate limit has been surpassed.
if you do receive an error indicating that you have exceeded your limit, the error message will tell you the duration you need to wait before resuming calls to that endpoint. Please take a look at the Terms of Use.
@ParthaI @rajlearner17 I see a total of 10 APIs that can be used to create the tables in - https://developer.godaddy.com/doc
Is there any specific reason why we didn't add other tables?