feat: add enableRetries and disableRetries #156

barrett-schonefeld · 2021-08-23T21:44:24Z

I fixed some type-related issues as well. I successfully resolved a couple of type-related issues (not bugs but improvements to type safety).

Checklist

npm test passes (tip: npm run lint-fix can correct most style issues)
tests are included
documentation is changed or added

padamstx

I left a bunch of picky comments about potentially changing variable names. Sorry for that, but I think it will help us maintain our own sanity if we're super clear about the two different types of "retry config".

lib/request-wrapper.ts

lib/base-service.ts

lib/request-wrapper.ts

dpopp07 · 2021-08-24T15:59:23Z

@barrett-schonefeld I sent this info on Slack but wanted to capture it here - the reason behind your is not a function problem is that the RequestWrapper class is mocked in the base service tests. See this block of code.

The code itself looks good, you will just need to update the mock for the tests accordingly. You can re-add your setter as well, you'll just need to do the same thing

dpopp07

Looks great! I left a couple comments and I'll wait for tests to approve but the code changes and looking solid 👍

test/unit/base-service.test.js

lib/request-wrapper.ts

…rvice

dpopp07

This looks great! I left a few comments on things to check before merging but nothing major

lib/request-wrapper.ts

test/mock-server/retry.test.js

padamstx

We seem to be getting closer and closer. Need to fix the value of retryDelay inside getRaxConfig(). I'll approve now assuming that's fixed.

lib/request-wrapper.ts

test/mock-server/retry.test.js

padamstx · 2021-08-25T23:47:45Z

Rut roh, i see that the build failed with lint errors :(

padamstx · 2021-08-25T23:48:33Z

... and I think you have more instances of that mock-related "this isn't a function" type errors :)

padamstx

Sorry, but I need to rescind my previous approval :) Please update BaseService.configureService() so that it supports the "ENABLE_RETRIES", "MAX_RETRIES" and "RETRY_INTERVAL" external config properties. You can look at BaseService.ConfigureService() in the Go core for a reference.
Also, we'll need to make sure those changes to configureService() are tested.

padamstx

Getting closer. We need some changes around the support of external config properties and I also requested some additional tests.

lib/base-service.ts

test/unit/base-service.test.js

padamstx

LGTM!

dpopp07

Looks good! 👍

# [2.14.0](v2.13.0...v2.14.0) (2021-08-27) ### Features * add enableRetries and disableRetries ([#156](#156)) ([d6f12e7](d6f12e7))

ibm-devx-sdk · 2021-08-27T20:02:11Z

🎉 This PR is included in version 2.14.0 🎉

The release is available on:

Your semantic-release bot 📦🚀

barrett-schonefeld requested review from dpopp07 and padamstx August 23, 2021 21:44

feat: add enableRetries and disableRetries

3c29f51

barrett-schonefeld force-pushed the retry-logic branch from 7c43144 to 3c29f51 Compare August 23, 2021 21:47

padamstx reviewed Aug 23, 2021

View reviewed changes

Barrett Schonefeld added 2 commits August 24, 2021 08:41

chore: refactor, convert seconds to ms, delete data in disableRetries()

8085663

test: ensure retry config is set up properly

ed85c17

dpopp07 reviewed Aug 24, 2021

View reviewed changes

test/unit/base-service.test.js Outdated Show resolved Hide resolved

lib/request-wrapper.ts Outdated Show resolved Hide resolved

lib/request-wrapper.ts Show resolved Hide resolved

Barrett Schonefeld added 2 commits August 24, 2021 11:45

test: add new functions to mock implementations and test raxConfig

4f2a87c

test: enable and disable retry unit tests

4198284

barrett-schonefeld force-pushed the retry-logic branch 2 times, most recently from a8912f5 to 242b345 Compare August 24, 2021 17:40

test: ensure enable and disable retries called correctly from base se…

9d16bcb

…rvice

barrett-schonefeld force-pushed the retry-logic branch from 242b345 to 9d16bcb Compare August 24, 2021 18:01

test: add acceptance tests for enable and disable retries

7d950e8

barrett-schonefeld force-pushed the retry-logic branch from 57e2456 to 7d950e8 Compare August 25, 2021 18:10

barrett-schonefeld requested review from dpopp07 and padamstx August 25, 2021 18:18

Barrett Schonefeld added 2 commits August 25, 2021 13:29

fix: better way to return data in result.result

2708fd7

chore: update retry defaults to align with other cores

1c4819f

dpopp07 approved these changes Aug 25, 2021

View reviewed changes

padamstx approved these changes Aug 25, 2021

View reviewed changes

lib/request-wrapper.ts Outdated Show resolved Hide resolved

test/mock-server/retry.test.js Outdated Show resolved Hide resolved

padamstx requested changes Aug 26, 2021

View reviewed changes

ricellis mentioned this pull request Aug 26, 2021

Support for retry mechanism in SDK IBM/cloudant-node-sdk#410

Closed

chore: respond to PR comments

badc918

barrett-schonefeld force-pushed the retry-logic branch from b1c7a5d to bdc49ed Compare August 26, 2021 17:08

barrett-schonefeld requested a review from padamstx August 26, 2021 17:08

barrett-schonefeld requested a review from dpopp07 August 26, 2021 17:08

feat: support enabling retries with environment variables

ab01872

barrett-schonefeld force-pushed the retry-logic branch from bdc49ed to ab01872 Compare August 26, 2021 17:12

padamstx requested changes Aug 26, 2021

View reviewed changes

lib/base-service.ts Outdated Show resolved Hide resolved

test/unit/base-service.test.js Outdated Show resolved Hide resolved

chore: test changes, external configuration changes

9bd7f29

barrett-schonefeld force-pushed the retry-logic branch from d359d18 to 9bd7f29 Compare August 26, 2021 22:25

barrett-schonefeld requested a review from padamstx August 26, 2021 22:31

test: reorganize tests

0bfd0c1

padamstx approved these changes Aug 26, 2021

View reviewed changes

dpopp07 approved these changes Aug 27, 2021

View reviewed changes

barrett-schonefeld merged commit d6f12e7 into main Aug 27, 2021

barrett-schonefeld deleted the retry-logic branch August 27, 2021 20:00

ibm-devx-sdk pushed a commit that referenced this pull request Aug 27, 2021

chore(release): 2.14.0 [skip ci]

524e183

# [2.14.0](v2.13.0...v2.14.0) (2021-08-27) ### Features * add enableRetries and disableRetries ([#156](#156)) ([d6f12e7](d6f12e7))

ibm-devx-sdk added the released label Aug 27, 2021

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat: add enableRetries and disableRetries #156

feat: add enableRetries and disableRetries #156

barrett-schonefeld commented Aug 23, 2021 •

edited

Loading

padamstx left a comment

dpopp07 commented Aug 24, 2021

dpopp07 left a comment

dpopp07 left a comment

padamstx left a comment

padamstx commented Aug 25, 2021

padamstx commented Aug 25, 2021

padamstx left a comment

padamstx left a comment

padamstx left a comment

dpopp07 left a comment

ibm-devx-sdk commented Aug 27, 2021

feat: add enableRetries and disableRetries #156

feat: add enableRetries and disableRetries #156

Conversation

barrett-schonefeld commented Aug 23, 2021 • edited Loading

Checklist

padamstx left a comment

Choose a reason for hiding this comment

dpopp07 commented Aug 24, 2021

dpopp07 left a comment

Choose a reason for hiding this comment

dpopp07 left a comment

Choose a reason for hiding this comment

padamstx left a comment

Choose a reason for hiding this comment

padamstx commented Aug 25, 2021

padamstx commented Aug 25, 2021

padamstx left a comment

Choose a reason for hiding this comment

padamstx left a comment

Choose a reason for hiding this comment

padamstx left a comment

Choose a reason for hiding this comment

dpopp07 left a comment

Choose a reason for hiding this comment

ibm-devx-sdk commented Aug 27, 2021

barrett-schonefeld commented Aug 23, 2021 •

edited

Loading