This repository was archived by the owner on Mar 3, 2026. It is now read-only.
WIP: retry strategy runner code#1567
Merged
shaffeeullah merged 17 commits intoSep 14, 2021
Merged
Conversation
Contributor
Author
|
@ddelgrosso1 would love some feedback here on the structure. there are multiple ways we can go about this and I need to think harder about what makes sense |
…gleapis/nodejs-storage into shaffeeullah/retryTestingRunnerCode
ddelgrosso1
reviewed
Sep 1, 2021
ddelgrosso1
reviewed
Sep 1, 2021
ddelgrosso1
reviewed
Sep 1, 2021
| "expectSuccess": false | ||
| } | ||
| ] | ||
| } No newline at end of file |
Contributor
There was a problem hiding this comment.
Probably get caught by the linter but this file is missing a new line.
Contributor
Author
There was a problem hiding this comment.
v4SignedUrl.json also doesn't have a new line. I think this is fine?
Contributor
Author
There was a problem hiding this comment.
or is new line the correct thing to do
Contributor
Author
|
@ddelgrosso1 Still WIP. Needs to be refactored for sure. I want to discuss the overall structure with you in our next sync before doing more. I'm not sure how I'm supposed to be using RESOURCES with the way that we structured the method mapping (and the way that I wrote the methods). Let's pair on this in our next sync. |
Contributor
|
@shaffeeullah sounds good to me. If I have some time today I will take a look again. |
…gleapis/nodejs-storage into shaffeeullah/retryTestingRunnerCode
…gleapis/nodejs-storage into shaffeeullah/retryTestingRunnerCode
ddelgrosso1
reviewed
Sep 14, 2021
| testCase.methods.forEach(jsonMethod => { | ||
| const functionList = methodMap.get(jsonMethod.name); | ||
| functionList?.forEach(storageMethodString => { | ||
| const storageMethodObject = (global as any).storageMethodString; |
Contributor
There was a problem hiding this comment.
If storageMethodString is coming in as string type you may have to use bracket notation here instead.
Contributor
Author
There was a problem hiding this comment.
using bracket notation throws errors: "Type string cannot be used as an index type"
I will run the code and debug it from there.
Contributor
There was a problem hiding this comment.
I would just make sure that using a . works with a string. Maybe TypeScript handles it cleaner than vanilla JS.
Contributor
There was a problem hiding this comment.
Actually, I think it might be because global is of type any and not object. If you have a way to test, I would just make sure dot notation works as expected and leave it that way if so.
Contributor
Author
There was a problem hiding this comment.
I was thinking of testing it after we merge this PR? i'm sure i'll encounter other bugs along the way.
Contributor
|
Structurally, I think this is in a good spot to go into the staging branch. Perhaps maybe just fix up the linter problems (at least the error)? |
Contributor
Author
Theoretically fixed the error. Waiting for linting to finish running. All the warnings saying that a function isn't used will continue to exist because the functions are used via variables. I will manually add an ignore to all of those. Let me know if you think this is the incorrect way to handle this situation. |
ddelgrosso1
approved these changes
Sep 14, 2021
ddelgrosso1
pushed a commit
to ddelgrosso1/nodejs-storage
that referenced
this pull request
Oct 4, 2021
* feat: retry strategy runner code * fixed license * removed unused option * 🦉 Updates from OwlBot See https://github.com/googleapis/repo-automation-bots/blob/main/packages/owl-bot/README.md * built out tests further, still doesnt work * 🦉 Updates from OwlBot See https://github.com/googleapis/repo-automation-bots/blob/main/packages/owl-bot/README.md * built out more functionality * 🦉 Updates from OwlBot See https://github.com/googleapis/repo-automation-bots/blob/main/packages/owl-bot/README.md * began refactor * 🦉 Updates from OwlBot See https://github.com/googleapis/repo-automation-bots/blob/main/packages/owl-bot/README.md * moved functions * refactored code * 🦉 Updates from OwlBot See https://github.com/googleapis/repo-automation-bots/blob/main/packages/owl-bot/README.md * Update libraryMethods.ts Co-authored-by: Owl Bot <gcf-owl-bot[bot]@users.noreply.github.com>
ddelgrosso1
pushed a commit
that referenced
this pull request
Oct 6, 2021
* feat: retry strategy runner code * fixed license * removed unused option * 🦉 Updates from OwlBot See https://github.com/googleapis/repo-automation-bots/blob/main/packages/owl-bot/README.md * built out tests further, still doesnt work * 🦉 Updates from OwlBot See https://github.com/googleapis/repo-automation-bots/blob/main/packages/owl-bot/README.md * built out more functionality * 🦉 Updates from OwlBot See https://github.com/googleapis/repo-automation-bots/blob/main/packages/owl-bot/README.md * began refactor * 🦉 Updates from OwlBot See https://github.com/googleapis/repo-automation-bots/blob/main/packages/owl-bot/README.md * moved functions * refactored code * 🦉 Updates from OwlBot See https://github.com/googleapis/repo-automation-bots/blob/main/packages/owl-bot/README.md * Update libraryMethods.ts Co-authored-by: Owl Bot <gcf-owl-bot[bot]@users.noreply.github.com>
ddelgrosso1
pushed a commit
to ddelgrosso1/nodejs-storage
that referenced
this pull request
Oct 13, 2021
* feat: retry strategy runner code * fixed license * removed unused option * 🦉 Updates from OwlBot See https://github.com/googleapis/repo-automation-bots/blob/main/packages/owl-bot/README.md * built out tests further, still doesnt work * 🦉 Updates from OwlBot See https://github.com/googleapis/repo-automation-bots/blob/main/packages/owl-bot/README.md * built out more functionality * 🦉 Updates from OwlBot See https://github.com/googleapis/repo-automation-bots/blob/main/packages/owl-bot/README.md * began refactor * 🦉 Updates from OwlBot See https://github.com/googleapis/repo-automation-bots/blob/main/packages/owl-bot/README.md * moved functions * refactored code * 🦉 Updates from OwlBot See https://github.com/googleapis/repo-automation-bots/blob/main/packages/owl-bot/README.md * Update libraryMethods.ts Co-authored-by: Owl Bot <gcf-owl-bot[bot]@users.noreply.github.com>
ddelgrosso1
added a commit
that referenced
this pull request
Oct 21, 2021
* feat: added library methods for conformance testing (#1553) * feat: added library methods for conformance testing * 🦉 Updates from OwlBot See https://github.com/googleapis/repo-automation-bots/blob/main/packages/owl-bot/README.md * fixed header * responded to PR comment, addressed some TODOs * resolved TODO * removed functions that dont make requests Co-authored-by: Owl Bot <gcf-owl-bot[bot]@users.noreply.github.com> * feat: add method mapping json file (#1569) * feat: add method mapping json file * add deleteResumableCache to mapping * WIP: retry strategy runner code (#1567) * feat: retry strategy runner code * fixed license * removed unused option * 🦉 Updates from OwlBot See https://github.com/googleapis/repo-automation-bots/blob/main/packages/owl-bot/README.md * built out tests further, still doesnt work * 🦉 Updates from OwlBot See https://github.com/googleapis/repo-automation-bots/blob/main/packages/owl-bot/README.md * built out more functionality * 🦉 Updates from OwlBot See https://github.com/googleapis/repo-automation-bots/blob/main/packages/owl-bot/README.md * began refactor * 🦉 Updates from OwlBot See https://github.com/googleapis/repo-automation-bots/blob/main/packages/owl-bot/README.md * moved functions * refactored code * 🦉 Updates from OwlBot See https://github.com/googleapis/repo-automation-bots/blob/main/packages/owl-bot/README.md * Update libraryMethods.ts Co-authored-by: Owl Bot <gcf-owl-bot[bot]@users.noreply.github.com> * fix: fixed runtime errors. all the tests still fail, but they run now (#1591) * fix: fixed runtime errors. all the tests still fail, but they run now * 🦉 Updates from OwlBot See https://github.com/googleapis/repo-automation-bots/blob/main/packages/owl-bot/README.md Co-authored-by: Owl Bot <gcf-owl-bot[bot]@users.noreply.github.com> * fix: exported functions * 🦉 Updates from OwlBot See https://github.com/googleapis/repo-automation-bots/blob/main/packages/owl-bot/README.md * test: pull testbench docker image, kokoro presubmit conformance test … (#1590) * test: pull testbench docker image, kokoro presubmit conformance test hooks * use async function instead of explicit promise return * move file to node12 directory * fix file naming and linter issues * fix: retry strategy bug fixes (#1594) * bug fixes * removed logs * 🦉 Updates from OwlBot See https://github.com/googleapis/repo-automation-bots/blob/main/packages/owl-bot/README.md * some tests pass * fixed some more tests * saves file * 🦉 Updates from OwlBot See https://github.com/googleapis/repo-automation-bots/blob/main/packages/owl-bot/README.md * uncommented functions * 🦉 Updates from OwlBot See https://github.com/googleapis/repo-automation-bots/blob/main/packages/owl-bot/README.md * removed duplicate storage definition * changed precondition options * down to 7 failing tests! * 🦉 Updates from OwlBot See https://github.com/googleapis/repo-automation-bots/blob/main/packages/owl-bot/README.md Co-authored-by: Owl Bot <gcf-owl-bot[bot]@users.noreply.github.com> * feat: check that instructions were utilized in testbench (#1601) * feat: check that instructions were utilized in testbench * feedback * fix: remove all files from bucket before delete (#1603) * feat: move test creation and header set to beforeEach (#1605) * feat: move test creation and header set to beforeEach * 🦉 Updates from OwlBot See https://github.com/googleapis/repo-automation-bots/blob/main/packages/owl-bot/README.md Co-authored-by: Denis DelGrosso <ddelgrosso@google.com> Co-authored-by: Owl Bot <gcf-owl-bot[bot]@users.noreply.github.com> * fix: create client for each test instead of globally (#1608) * fix: remove bucketGet (#1610) * fix: return promise from stream functions and resolve on end (#1612) * fix: move createReadStream to correct mapping location (#1613) * fix: remap bucket set storage class and bucket set metadata (#1615) * fix: remap bucket set storage class * fix: remap bucketSetMetadata * fix: fixed bucketUploadMultipart (#1627) * fix: fixed bucketUploadMultipart * 🦉 Updates from OwlBot See https://github.com/googleapis/repo-automation-bots/blob/main/packages/owl-bot/README.md Co-authored-by: Owl Bot <gcf-owl-bot[bot]@users.noreply.github.com> * fix: conformance test fixes for deleteFiles, fileDelete, and copy (#1636) * fix: add copy to invocation map * fix: fix deleteFiles and fileDelete conformance tests * linter fixes * fix: only send metageneration to bucket related tests (#1641) * fix: remove tests for createWriteStream as the function is not retryable * fix: only send metageneration to bucket related tests * linter * fix: set retention period before file creation (#1651) * set retention period before file creation * removed log statement * removed log statement * fix: set correct bucket precondition (#1653) * fix: set correct bucket precondition * Update retryStrategy.ts * 🦉 Updates from OwlBot See https://github.com/googleapis/repo-automation-bots/blob/main/packages/owl-bot/README.md Co-authored-by: Owl Bot <gcf-owl-bot[bot]@users.noreply.github.com> * fix: combine conformance test (#1656) * fix: update test bucket (#1657) * fix: combine conformance test * fix: only return bucket with preconditions if the test says so * 🦉 Updates from OwlBot See https://github.com/googleapis/repo-automation-bots/blob/main/packages/owl-bot/README.md Co-authored-by: Owl Bot <gcf-owl-bot[bot]@users.noreply.github.com> * fix: fixed storage.objects.patch tests (#1658) * fix: fixed storage.objects.patch tests * 🦉 Updates from OwlBot See https://github.com/googleapis/repo-automation-bots/blob/main/packages/owl-bot/README.md Co-authored-by: Owl Bot <gcf-owl-bot[bot]@users.noreply.github.com> * fix: refactor tests (#1659) * fix: refactor tests * 🦉 Updates from OwlBot See https://github.com/googleapis/repo-automation-bots/blob/main/packages/owl-bot/README.md Co-authored-by: Owl Bot <gcf-owl-bot[bot]@users.noreply.github.com> * chore: build headers for isPublic from interceptors (#1665) * chore: build headers for isPublic from interceptors * simplify code * tests: removed failing tests (internal issue b/203793664) (#1674) * fix: add conformance project id (#1673) * fix: add conformance test project id settting * lint * increase timeout for before hook * specify correct host to docker port mapping * add stop command for docker container * chore: use import on json files instead of fs read (#1675) Co-authored-by: Owl Bot <gcf-owl-bot[bot]@users.noreply.github.com> Co-authored-by: Denis DelGrosso <85250797+ddelgrosso1@users.noreply.github.com> Co-authored-by: Denis DelGrosso <ddelgrosso@google.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Not close to being done/working.