IBM / go-sdk-core

The go-sdk-core repository contains core functionality required by Go code generated by the IBM OpenAPI SDK Generator.
Apache License 2.0
30 stars 24 forks source link

feat: introduce support for automatic retries #82

Closed padamstx closed 3 years ago

padamstx commented 3 years ago

Fixes arf/planning-sdk-squad#2229

This commit includes:

In order to get a good overall view of what's going on during the tests, I suggest that reviewers pull the feature branch and then run the retry-related tests with cd v4/core; go test -ginkgo.v -tags=retries. That will show the various retries being performed and the resulting status code for each request.

Re: the logger, my intent was to provide a basic logger that supports various logging levels that in turn uses the underlying log.Logger facility provided by the Go engine. I define the core.Logger interface so that anyone could implement their own and supply that as the Logger instance used by the core. This might be useful in CLI and/or Terraform contexts.

padamstx commented 3 years ago

cc: @smikulcik please provide feedback if you want :)

padamstx commented 3 years ago

This looks good. Though I pulled the branch and ran the tests locally and got the following test failure:

  Expected
      <string>: Get http://notahost:12345: dial tcp 23.217.138.110:12345: connect: connection refused
  to contain substring
      <string>: no such host

Aha! It looks like I was being too specific in the error message returned for an "invalid hostname" situation. This is likely just differences in the TCP/IP implementations on linux vs MacOS. I'll just relax the checks a bit in the unit tests.

padamstx commented 3 years ago

We should also close https://github.ibm.com/arf/planning-sdk-squad/issues/2079 if we feel like this PR satisfied the requirements for adding a logger to go.

While I've added a basic logging capability to the Go core to support the retryable request processing stuff, I think we want to keep that issue open so that we can add logging steps where needed throughout the Go core code to log errors and perhaps some debug or info messages at strategic places.

ambron60 commented 3 years ago

Greetings.. what is this 23.217.138.110 Akamai host and what is p 22 being used for?

padamstx commented 3 years ago

Greetings.. what is this 23.217.138.110 Akamai host and what is p 22 being used for?

Hi @ambron60, not sure what this question is pertaining to. Could you be more specific?

ambron60 commented 3 years ago

Hi @padamstx and thanks for taking my question.. one of our hosts was resolving to this particular IP and I can't find any info/reference to it except for its mapping to Akamai- I saw you guys post above making reference to it, so I was inclined to ask.. it also looks like it has ports 22 and 80 open, not sure why.

Ex IBM-er here btw.. 👍

padamstx commented 3 years ago

Hi @padamstx and thanks for taking my question.. one of our hosts was resolving to this particular IP and I can't find any info/reference to it except for its mapping to Akamai- I saw you guys post above making reference to it, so I was inclined to ask.. it also looks like it has ports 22 and 80 open, not sure why.

Ah, now I see the connection. I think it was just a coincidence that one of our testcases uses hostname "notahost" in a negative test, and that apparently resolved to the IP address, but I'm not sure how that happened to be honest. This PR really has no connection to Akamai at all.

ambron60 commented 3 years ago

Hi @padamstx and thanks for taking my question.. one of our hosts was resolving to this particular IP and I can't find any info/reference to it except for its mapping to Akamai- I saw you guys post above making reference to it, so I was inclined to ask.. it also looks like it has ports 22 and 80 open, not sure why.

Ah, now I see the connection. I think it was just a coincidence that one of our testcases uses hostname "notahost" in a negative test, and that apparently resolved to the IP address, but I'm not sure how that happened to be honest. This PR really has no connection to Akamai at all.

Thanks!!

ibm-devx-automation commented 3 years ago

:tada: This PR is included in version 4.8.0 :tada:

The release is available on GitHub release

Your semantic-release bot :package::rocket: