aws-lambda: Add SQS BatchItemFailures support #57488

bboure · 2021-12-01T22:53:15Z

Please fill in this template.

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.) [TODO]
Add or edit tests to reflect the change.
Follow the advice from the readme.
Avoid common mistakes.
Run npm test <package to test>.

If changing an existing definition:

Provide a URL to documentation or source code which provides context for the suggested changes: <>

bboure · 2021-12-01T23:41:50Z

Experienced some issues with tests failing.

Found the reason

https://github.com/microsoft/DefinitelyTyped-tools/pull/361/files

cc: @sandersn

sandersn · 2021-12-02T00:50:08Z

I broke CI tests this morning when I updated the @definitelytyped packages. It is fixed now, but you'll have to merge from master to get the tslint changes to make tests pass.

bboure · 2021-12-02T08:09:56Z

Still failing with

Could not find config file at: /home/runner/work/DefinitelyTyped/DefinitelyTyped/node_modules/@definitelytyped/dtslint/bin/dtslint.json

Any idea?

sandersn · 2021-12-02T23:02:38Z

There was another bug for packages with dependents: microsoft/DefinitelyTyped-tools#362. That's fixed now, so I'll re-run the tests by closing/re-opening the PR.

maplesteve · 2021-12-03T01:00:28Z

types/aws-lambda/trigger/sqs.d.ts

@@ -44,3 +44,12 @@ export interface SQSMessageAttribute {
 export interface SQSMessageAttributes {
    [name: string]: SQSMessageAttribute;
 }
+
+// https://docs.aws.amazon.com/lambda/latest/dg/with-sqs.html#services-sqs-batchfailurereporting
+export interface SQSEventResponse {


This is a response dedicated for batches. so I would suggest to make this clear in the naming. SQSBatchResponse is used by the aws-sdk.

Suggested change

export interface SQSEventResponse {

export interface SQSBatchResponse {

maplesteve · 2021-12-03T01:01:40Z

types/aws-lambda/trigger/sqs.d.ts

+    batchItemFailures: SQSBatchItemFailures[];
+}
+
+export interface SQSBatchItemFailures {


This interface describes only one failure

Suggested change

export interface SQSBatchItemFailures {

export interface SQSBatchItemFailure {

Fix Fix tests Fix

typescript-bot · 2021-12-04T15:41:58Z

@bboure Thank you for submitting this PR!

This is a live comment which I will keep updated.

1 package in this PR

aws-lambda — on npm, on unpkg (author is owner)
- owner-approval: @jamesorlakin

Code Reviews

Because you edited one package and updated the tests (👏), I can help you merge this PR once someone else signs off on it.

You can test the changes of this PR in the Playground.

Status

✅ No merge conflicts
✅ Continuous integration tests have passed
✅ Most recent commit is approved by type definition owners or DT maintainers

All of the items on the list are green. To merge, you need to post a comment including the string "Ready to merge" to bring in your changes.

Diagnostic Information: What the bot saw about this PR

{
  "type": "info",
  "now": "-",
  "pr_number": 57488,
  "author": "bboure",
  "headCommitOid": "a6fb5538015d298e0c99ad35a4c4212d901bd614",
  "lastPushDate": "2021-12-06T11:42:41.000Z",
  "lastActivityDate": "2021-12-06T12:10:51.000Z",
  "mergeOfferDate": "2021-12-06T12:03:03.000Z",
  "mergeRequestDate": "2021-12-06T12:10:51.000Z",
  "mergeRequestUser": "bboure",
  "hasMergeConflict": false,
  "isFirstContribution": false,
  "tooManyFiles": false,
  "hugeChange": false,
  "popularityLevel": "Popular",
  "pkgInfo": [
    {
      "name": "aws-lambda",
      "kind": "edit",
      "files": [
        {
          "path": "types/aws-lambda/test/sqs-tests.ts",
          "kind": "test"
        },
        {
          "path": "types/aws-lambda/trigger/sqs.d.ts",
          "kind": "definition"
        }
      ],
      "owners": [
        "darbio",
        "skarum",
        "StefH",
        "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",
        "apalumbo",
        "SachinShekhar",
        "ivanmartos",
        "zach-anthony",
        "savnik",
        "svenmilewski",
        "bboure",
        "jamesorlakin"
      ],
      "addedOwners": [],
      "deletedOwners": [],
      "popularityLevel": "Popular"
    }
  ],
  "reviews": [
    {
      "type": "approved",
      "reviewer": "jamesorlakin",
      "date": "2021-12-06T12:02:26.000Z",
      "isMaintainer": false
    },
    {
      "type": "stale",
      "reviewer": "maplesteve",
      "date": "2021-12-04T16:41:00.000Z",
      "abbrOid": "f41426a"
    }
  ],
  "mainBotCommentID": 986047288,
  "ciResult": "pass"
}

typescript-bot · 2021-12-04T15:42:00Z

🔔 @darbio @skarum @StefH @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 @apalumbo @SachinShekhar @ivanmartos @zach-anthony @savnik @svenmilewski @jamesorlakin — 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.

typescript-bot · 2021-12-04T15:47:03Z

@maplesteve 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?

maplesteve

👍

jamesorlakin · 2021-12-06T11:20:20Z

types/aws-lambda/tslint.json

@@ -1,6 +1,7 @@
 {
    "extends": "@definitelytyped/dtslint/dt.json",
    "rules": {
-        "npm-naming": false
+        "npm-naming": false,
+        "void-return": false


DefinitelyTyped suggests not to add any special blanket rules for packages. I agree that the rule shouldn't be triggering for our usecase as the suggested undefined replacement does not work when I tried locally.

With that in mind I think disabling it just as a one-off is probably the more sanctioned approach:

// tslint:disable-next-line:void-return export type SQSHandler = Handler<SQSEvent, SQSBatchResponse | void>;

Yeah, sounds good to me.

typescript-bot · 2021-12-06T11:48:34Z

@jamesorlakin, @maplesteve 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?

jamesorlakin

Looks good to me, thanks for adding!

typescript-bot · 2021-12-06T12:03:03Z

@bboure: Everything looks good here. I am ready to merge this PR (at a6fb553) on your behalf whenever you think it's ready.

If you'd like that to happen, please post a comment saying:

Ready to merge

and I'll merge this PR almost instantly. Thanks for helping out! ❤️

(@darbio, @skarum, @StefH, @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, @apalumbo, @SachinShekhar, @ivanmartos, @zach-anthony, @savnik, @svenmilewski, @jamesorlakin: you can do this too.)

typescript-bot · 2021-12-06T12:03:04Z

@maplesteve 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?

bboure · 2021-12-06T12:10:51Z

Ready to merge

typescript-bot added this to Needs Author Action in New Pull Request Status Board Dec 1, 2021

sandersn closed this Dec 2, 2021

sandersn reopened this Dec 2, 2021

maplesteve reviewed Dec 3, 2021

View reviewed changes

feat: Add SQS BatchItemFailures support

f41426a

Fix Fix tests Fix

bboure force-pushed the feat/sqs-batchfailures branch from 6df638f to f41426a Compare December 4, 2021 15:40

bboure marked this pull request as ready for review December 4, 2021 15:41

typescript-bot added Popular package This PR affects a popular package (as counted by NPM download counts). Author is Owner The author of this PR is a listed owner of the package. Check Config Changes a module config files labels Dec 4, 2021

typescript-bot moved this from Needs Author Action to Waiting for Code Reviews in New Pull Request Status Board Dec 4, 2021

typescript-bot moved this from Waiting for Code Reviews to Needs Maintainer Review in New Pull Request Status Board Dec 4, 2021

maplesteve approved these changes Dec 4, 2021

View reviewed changes

typescript-bot added the Other Approved This PR was reviewed and signed-off by a community member. label Dec 4, 2021

jamesorlakin reviewed Dec 6, 2021

View reviewed changes

Fix lint

a6fb553

typescript-bot removed Other Approved This PR was reviewed and signed-off by a community member. Check Config Changes a module config files labels Dec 6, 2021

typescript-bot moved this from Needs Maintainer Review to Waiting for Code Reviews in New Pull Request Status Board Dec 6, 2021

jamesorlakin approved these changes Dec 6, 2021

View reviewed changes

typescript-bot added the Owner Approved A listed owner of this package signed off on the pull request. label Dec 6, 2021

typescript-bot added the Self Merge This PR can now be self-merged by the PR author or an owner label Dec 6, 2021

typescript-bot moved this from Waiting for Code Reviews to Waiting for Author to Merge in New Pull Request Status Board Dec 6, 2021

typescript-bot moved this from Waiting for Author to Merge to Recently Merged in New Pull Request Status Board Dec 6, 2021

typescript-bot merged commit c183617 into DefinitelyTyped:master Dec 6, 2021

typescript-bot removed this from Recently Merged in New Pull Request Status Board Dec 6, 2021

armanbilge mentioned this pull request Dec 9, 2021

Add SQSEvent typelevel/feral#58

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

aws-lambda: Add SQS BatchItemFailures support #57488

aws-lambda: Add SQS BatchItemFailures support #57488

bboure commented Dec 1, 2021 •

edited

Loading

bboure commented Dec 1, 2021

sandersn commented Dec 2, 2021

bboure commented Dec 2, 2021

sandersn commented Dec 2, 2021

maplesteve Dec 3, 2021

maplesteve Dec 3, 2021

typescript-bot commented Dec 4, 2021 •

edited

Loading

typescript-bot commented Dec 4, 2021

typescript-bot commented Dec 4, 2021

maplesteve left a comment

jamesorlakin Dec 6, 2021

bboure Dec 6, 2021

typescript-bot commented Dec 6, 2021

jamesorlakin left a comment

typescript-bot commented Dec 6, 2021

typescript-bot commented Dec 6, 2021

bboure commented Dec 6, 2021

	export interface SQSEventResponse {
	export interface SQSBatchResponse {

	export interface SQSBatchItemFailures {
	export interface SQSBatchItemFailure {

aws-lambda: Add SQS BatchItemFailures support #57488

aws-lambda: Add SQS BatchItemFailures support #57488

Conversation

bboure commented Dec 1, 2021 • edited Loading

bboure commented Dec 1, 2021

sandersn commented Dec 2, 2021

bboure commented Dec 2, 2021

sandersn commented Dec 2, 2021

maplesteve Dec 3, 2021

Choose a reason for hiding this comment

maplesteve Dec 3, 2021

Choose a reason for hiding this comment

typescript-bot commented Dec 4, 2021 • edited Loading

1 package in this PR

Code Reviews

Status

typescript-bot commented Dec 4, 2021

typescript-bot commented Dec 4, 2021

maplesteve left a comment

Choose a reason for hiding this comment

jamesorlakin Dec 6, 2021

Choose a reason for hiding this comment

bboure Dec 6, 2021

Choose a reason for hiding this comment

typescript-bot commented Dec 6, 2021

jamesorlakin left a comment

Choose a reason for hiding this comment

typescript-bot commented Dec 6, 2021

typescript-bot commented Dec 6, 2021

bboure commented Dec 6, 2021

bboure commented Dec 1, 2021 •

edited

Loading

typescript-bot commented Dec 4, 2021 •

edited

Loading