Added requestId member on all CloudFrontEvent types #43696

nhodin · 2020-04-07T08:06:54Z

The requestId member is defined on all origin/viewer requests/responses, it is described here : https://docs.aws.amazon.com/AmazonCloudFront/latest/DeveloperGuide/lambda-event-structure.html
There is one example for each request/response and they all have a cf.config.requestId.
Added new requests examples in tests in order to have all 4 types availabl

Use a meaningful title for the pull request. Include the name of the package modified.
Test the change in your own code. (Compile and run.)
Add or edit tests to reflect the change. (Run with npm test.)
Follow the advice from the readme.
Avoid common mistakes.
Run npm run lint package-name (or tsc if no tslint.json is present).

If changing an existing definition:

Provide a URL to documentation or source code which provides context for the suggested changes: https://docs.aws.amazon.com/AmazonCloudFront/latest/DeveloperGuide/lambda-event-structure.html
If this PR brings the type definitions up to date with a new version of the JS library, update the version number in the header.
If you are making substantial changes, consider adding a tslint.json containing { "extends": "dtslint/dt.json" }. If for reason the any rule need to be disabled, disable it for that line using // tslint:disable-next-line [ruleName] and not for whole package so that the need for disabling can be reviewed.

The requestId member is defined on all origin/viewer requests/responses, it is described here : https://docs.aws.amazon.com/AmazonCloudFront/latest/DeveloperGuide/lambda-event-structure.html There is one example for each request/response and they all have a cf.config.requestId. Added new requests examples in tests in order to have all 4 types available

typescript-bot · 2020-04-07T08:07:03Z

@nhodin Thank you for submitting this PR!

Code Reviews

Because this is a widely-used package, a DT maintainer will need to review it before it can be merged.

🔔 @darbio @skarum @StefH @tobyhede @buggy @y13i @wwwy3y3 @OrthoDex @MichaelMarner @daniel-cottone @kostya-misura @coderbyheart @palmithor @daniloraisi @simonbuchan @Haydabase @repl-chris @aneilbaboo @jeznag @louislarry @dpapukchiev @ohookins @trevor-leach @jagregory @dalen @loikg @skyzenr @redlickigrzegorz @juancarbonel @pwmcintyre @alex-bolenok-centralreach @marianzange @apepper - please review this PR in the next few days. Be sure to explicitly select Approve or Request Changes in the GitHub UI so I know what's going on.

Status

✅ No merge conflicts
✅ Continuous integration tests have passed
✅ Most recent commit is approved by a DT maintainer

Once every item on this list is checked, I'll ask you for permission to merge and publish the changes.

Diagnostic Information: What the bot saw about this PR

{
  "type": "info",
  "pr_number": 43696,
  "author": "nhodin",
  "owners": [
    "darbio",
    "skarum",
    "StefH",
    "tobyhede",
    "buggy",
    "y13i",
    "wwwy3y3",
    "OrthoDex",
    "MichaelMarner",
    "daniel-cottone",
    "kostya-misura",
    "coderbyheart",
    "palmithor",
    "daniloraisi",
    "simonbuchan",
    "Haydabase",
    "repl-chris",
    "aneilbaboo",
    "jeznag",
    "louislarry",
    "dpapukchiev",
    "ohookins",
    "trevor-leach",
    "jagregory",
    "dalen",
    "loikg",
    "skyzenr",
    "redlickigrzegorz",
    "juancarbonel",
    "pwmcintyre",
    "alex-bolenok-centralreach",
    "marianzange",
    "apepper"
  ],
  "dangerLevel": "ScopedAndTested",
  "headCommitAbbrOid": "6564ad3",
  "headCommitOid": "6564ad32b7a2e1a1eed06cb419b3cf26582bc970",
  "mergeIsRequested": true,
  "stalenessInDays": 1,
  "lastCommitDate": "2020-04-07T13:57:39.000Z",
  "reviewLink": "https://github.com/DefinitelyTyped/DefinitelyTyped/pull/43696/files",
  "hasMergeConflict": false,
  "authorIsOwner": false,
  "isFirstContribution": false,
  "popularityLevel": "Popular",
  "anyPackageIsNew": false,
  "packages": [
    "aws-lambda"
  ],
  "files": [
    {
      "filePath": "types/aws-lambda/common/cloudfront.d.ts",
      "kind": "definition",
      "package": "aws-lambda"
    },
    {
      "filePath": "types/aws-lambda/test/cloudfront-tests.ts",
      "kind": "test",
      "package": "aws-lambda"
    }
  ],
  "otherApprovalCount": 0,
  "ownerApprovalCount": 2,
  "maintainerApprovalCount": 0,
  "hasDismissedReview": false,
  "travisResult": "pass",
  "reviewersWithStaleReviews": [],
  "approvalFlags": 2,
  "isChangesRequested": false
}

typescript-bot · 2020-04-07T08:22:28Z

@nhodin Thank you for submitting this PR!

🔔 @darbio @skarum @StefH @tobyhede @buggy @y13i @wwwy3y3 @OrthoDex @MichaelMarner @daniel-cottone @kostya-misura @coderbyheart @palmithor @daniloraisi @simonbuchan @Haydabase @repl-chris @aneilbaboo @jeznag @louislarry @dpapukchiev @ohookins @trevor-leach @jagregory @dalen @loikg @skyzenr @redlickigrzegorz @juancarbonel @pwmcintyre @alex-bolenok-centralreach @marianzange @apepper - please review this PR in the next few days. Be sure to explicitly select Approve or Request Changes in the GitHub UI so I know what's going on.

If no reviewer appears after a week, a DefinitelyTyped maintainer will review the PR instead.

simonbuchan

Looks good. I checked with the original PR that added them (#40355) to see the justification, and it specifically calls out the same documentation with "requestId is only available for viewer events", so it looks like either the docs were wrong or cloudfront has been updated since.

simonbuchan · 2020-04-07T09:10:27Z

types/aws-lambda/common/cloudfront.d.ts

-    } & (
-        | { readonly eventType: 'origin-request' | 'origin-response' }
-        | { readonly eventType: 'viewer-request' | 'viewer-response'; readonly requestId: string });
+        readonly eventType: 'origin-request' | 'origin-response'|'viewer-request' | 'viewer-response';


completely trivial nit: spacing around the middle |

Indeed, sorry... I am able to fix this before the merge ?

It may require me to re-approve, but that's no problem.

typescript-bot · 2020-04-07T09:22:37Z

@nhodin Everything looks good here. Great job! I am ready to merge this PR on your behalf.
If you'd like that to happen, please post a comment with the exact text

Ready to merge

and I'll merge it the next time I look at this PR.

typescript-bot · 2020-04-07T09:22:37Z

A definition owner has approved this PR ⭐️. A maintainer will merge this PR shortly. If it shouldn't be merged yet, please leave a comment saying so and we'll wait. Thank you for your contribution to DefinitelyTyped!

typescript-bot · 2020-04-07T09:22:37Z

@nhodin Everything looks good here. Great job! I am ready to merge this PR on your behalf.
If you'd like that to happen, please post a comment with the exact text

Ready to merge

and I'll merge it the next time I look at this PR.

typescript-bot · 2020-04-07T14:23:00Z

@simonbuchan Thank you for reviewing this PR! The author has pushed new commits since your last review. Could you take another look and submit a fresh review?

typescript-bot · 2020-04-07T14:23:00Z

@simonbuchan Thank you for reviewing this PR! The author has pushed new commits since your last review. Could you take another look and submit a fresh review?

simonbuchan

Looks like the bot has changed a little, and you can merge it yourself now! It also looks like it's detected there's isn't another review necessary, but just in case.

nhodin · 2020-04-08T08:38:53Z

Looks like the bot has changed a little, and you can merge it yourself now! It also looks like it's detected there's isn't another review necessary, but just in case.

Thanks @simonbuchan ! But i don't find any Merge button in this page :( Is there another way to do it ?

orta · 2020-04-08T17:22:39Z

The bot is very new - and we're working out the copy and issues ATM. Can try following the directions in #43696 (comment) please @nhodin

nhodin · 2020-04-08T17:58:33Z

Ready to merge

nhodin · 2020-04-08T18:00:51Z

@orta thanks for your help, sorry that I didn't see that comment earlier..

orta · 2020-04-08T18:02:56Z

You're welcome - there's a lot of noise, but I think this is the first from "0 -> Self Merge" we've seen.

RyanCavanaugh · 2020-04-08T18:04:02Z

🎉

typescript-bot · 2020-04-08T18:05:05Z

I just published @types/aws-lambda@8.10.48 to npm.

nhodin added 2 commits April 7, 2020 09:51

Fix lint issue with consecutive empty lines

1126bee

typescript-bot added Where is Travis? Popular package This PR affects a popular package (as counted by NPM download counts). labels Apr 7, 2020

typescript-bot added this to Other in New Pull Request Status Board Apr 7, 2020

typescript-bot added this to Waiting for Reviewers in Pull Request Status Board Apr 7, 2020

typescript-bot added Awaiting reviewer feedback and removed Where is Travis? labels Apr 7, 2020

typescript-bot moved this from Other to Waiting for Code Reviews in New Pull Request Status Board Apr 7, 2020

simonbuchan approved these changes Apr 7, 2020

View reviewed changes

typescript-bot moved this from Waiting for Reviewers to Check and Merge in Pull Request Status Board Apr 7, 2020

typescript-bot added Owner Approved A listed owner of this package signed off on the pull request. Merge:Express and removed Awaiting reviewer feedback labels Apr 7, 2020

typescript-bot moved this from Waiting for Code Reviews to Waiting for Author to Merge in New Pull Request Status Board Apr 7, 2020

Fix missing spaces

6564ad3

typescript-bot added Where is Travis? and removed Owner Approved A listed owner of this package signed off on the pull request. labels Apr 7, 2020

typescript-bot moved this from Waiting for Author to Merge to Other in New Pull Request Status Board Apr 7, 2020

typescript-bot added Owner Approved A listed owner of this package signed off on the pull request. and removed Where is Travis? labels Apr 7, 2020

typescript-bot added Owner Approved A listed owner of this package signed off on the pull request. and removed Owner Approved A listed owner of this package signed off on the pull request. labels Apr 7, 2020

simonbuchan approved these changes Apr 8, 2020

View reviewed changes

typescript-bot added the Owner Approved A listed owner of this package signed off on the pull request. label Apr 8, 2020

typescript-bot moved this from Waiting for Code Reviews to Waiting for Author to Merge in New Pull Request Status Board Apr 8, 2020

typescript-bot moved this from Waiting for Author to Merge to Recently Merged in New Pull Request Status Board Apr 8, 2020

typescript-bot merged commit c72d502 into DefinitelyTyped:master Apr 8, 2020

Pull Request Status Board automation moved this from Check and Merge to Done Apr 8, 2020

orta mentioned this pull request Apr 8, 2020

Bot doesn't get web hooks for new issue comments (nor new reviews) DefinitelyTyped/dt-mergebot#2

Closed

typescript-bot removed this from Done in Pull Request Status Board Apr 8, 2020

orta mentioned this pull request Apr 8, 2020

Revise the messaging for when the bot can merge your PR DefinitelyTyped/dt-mergebot#6

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Added requestId member on all CloudFrontEvent types #43696

Added requestId member on all CloudFrontEvent types #43696

nhodin commented Apr 7, 2020 •

edited by orta

typescript-bot commented Apr 7, 2020 •

edited

typescript-bot commented Apr 7, 2020 •

edited

simonbuchan left a comment

simonbuchan Apr 7, 2020

nhodin Apr 7, 2020

simonbuchan Apr 7, 2020

typescript-bot commented Apr 7, 2020

typescript-bot commented Apr 7, 2020

typescript-bot commented Apr 7, 2020

typescript-bot commented Apr 7, 2020

typescript-bot commented Apr 7, 2020

simonbuchan left a comment

nhodin commented Apr 8, 2020 •

edited

orta commented Apr 8, 2020

nhodin commented Apr 8, 2020

nhodin commented Apr 8, 2020

orta commented Apr 8, 2020

RyanCavanaugh commented Apr 8, 2020

typescript-bot commented Apr 8, 2020

Added requestId member on all CloudFrontEvent types #43696

Added requestId member on all CloudFrontEvent types #43696

Conversation

nhodin commented Apr 7, 2020 • edited by orta

typescript-bot commented Apr 7, 2020 • edited

Code Reviews

Status

typescript-bot commented Apr 7, 2020 • edited

simonbuchan left a comment

Choose a reason for hiding this comment

simonbuchan Apr 7, 2020

Choose a reason for hiding this comment

nhodin Apr 7, 2020

Choose a reason for hiding this comment

simonbuchan Apr 7, 2020

Choose a reason for hiding this comment

typescript-bot commented Apr 7, 2020

typescript-bot commented Apr 7, 2020

typescript-bot commented Apr 7, 2020

typescript-bot commented Apr 7, 2020

typescript-bot commented Apr 7, 2020

simonbuchan left a comment

Choose a reason for hiding this comment

nhodin commented Apr 8, 2020 • edited

orta commented Apr 8, 2020

nhodin commented Apr 8, 2020

nhodin commented Apr 8, 2020

orta commented Apr 8, 2020

RyanCavanaugh commented Apr 8, 2020

typescript-bot commented Apr 8, 2020

nhodin commented Apr 7, 2020 •

edited by orta

typescript-bot commented Apr 7, 2020 •

edited

typescript-bot commented Apr 7, 2020 •

edited

nhodin commented Apr 8, 2020 •

edited