search

blimmer / cdk-datadog-integration

Amazon Cloud Development Kit (CDK) logic to integrate your AWS account with Datadog
Apache License 2.0
19 stars 8 forks source link

Looser peerDependencies for CDK modules #23

Closed plumdog closed 3 years ago

plumdog commented 3 years ago

While I don't believe this is totally accurate, as such, I suspect that there are CDK versions below 1.64 where this module will work fine, but it looks like npm is now stricter when peer dependencies are not satisfied.

Attempting to install this module where I'm using an older version of CDK, I get an error like:

$ npm i --save cdk-datadog-integration
npm ERR! code ERESOLVE
npm ERR! ERESOLVE unable to resolve dependency tree
npm ERR! 
npm ERR! While resolving: myproject@1.0.0
npm ERR! Found: @aws-cdk/aws-cloudwatch@1.102.0
npm ERR! node_modules/@aws-cdk/aws-cloudwatch
npm ERR!   peer @aws-cdk/aws-cloudwatch@"1.102.0" from @aws-cdk/aws-lambda@1.102.0
npm ERR!   node_modules/@aws-cdk/aws-lambda
npm ERR!     peer @aws-cdk/aws-lambda@"1.102.0" from @aws-cdk/aws-cloudformation@1.102.0
npm ERR!     node_modules/@aws-cdk/aws-cloudformation
npm ERR!       peer @aws-cdk/aws-cloudformation@"^1.64.1" from cdk-datadog-integration@1.1.1
npm ERR!       node_modules/cdk-datadog-integration
npm ERR!         cdk-datadog-integration@"*" from the root project
npm ERR!   peer @aws-cdk/aws-cloudwatch@"1.102.0" from @aws-cdk/aws-applicationautoscaling@1.102.0
npm ERR!   node_modules/@aws-cdk/aws-applicationautoscaling
npm ERR!     peer @aws-cdk/aws-applicationautoscaling@"1.102.0" from @aws-cdk/aws-lambda@1.102.0
npm ERR!     node_modules/@aws-cdk/aws-lambda
npm ERR!       peer @aws-cdk/aws-lambda@"1.102.0" from @aws-cdk/aws-cloudformation@1.102.0
npm ERR!       node_modules/@aws-cdk/aws-cloudformation
npm ERR!         peer @aws-cdk/aws-cloudformation@"^1.64.1" from cdk-datadog-integration@1.1.1
npm ERR!         node_modules/cdk-datadog-integration
npm ERR! 
npm ERR! Could not resolve dependency:
npm ERR! peer @aws-cdk/aws-cloudwatch@"1.55.0" from @aws-cdk/aws-ec2@1.55.0
npm ERR! node_modules/@aws-cdk/aws-ec2
npm ERR!   @aws-cdk/aws-ec2@"1.55.0" from the root project

That is, npm is - not unreasonably - complaining because my package.json has "@aws-cdk/aws-ec2": "1.55.0" but this module has "@aws-cdk/aws-cloudformation": "^1.64.1", and each of those wants a compatible @aws-cdk/aws-cloudwatch, which are incompatible. It looks like this is different behaviour between npm 6 and 7, where 6 was more lenient, but 7 errors.

One option would be for me to upgrade CDK. In my case that is not possible. Another would be to update this modules package.json to have the loosest functioning peer dependencies, but alas, I'm just not that diligent. Thus, I have arrived at the changes that I have here.

blimmer commented 3 years ago

Hey @plumdog - thanks for this PR. I'm sure there's some version earlier than 1.64.1 that breaks this library but I'm OK with loosening this requirement as long as there's a note about compatibility.

Would you be willing to do two things with this PR?

  1. Add a comment in README.md that this library is tested with 1.xx (1.55.0 if you've tested against it, 1.64.1 if not). A disclaimer should be included that "your mileage may vary" with versions earlier than the tested version.
  2. Add <2 to the peer dependencies while we're updating it. v2 is a breaking change and I'm assuming I'll need to do some updates to be compatible when it's released.

Thanks for your contribution!

plumdog commented 3 years ago

It does indeed work fine with 1.55.0, based on my testing - which is to say: it successfully creates the stacks I expect, and metrics and logs are arriving in Datadog, so I consider that "passed testing". I've added a note to the README to say as much.

I believe that the version constraints already in package.json do exclude a new major version. NPM's docs have the example:

^1.2.3 := >=1.2.3 <2.0.0

so I think the version constraint is already set to the right thing. See https://docs.npmjs.com/cli/v7/using-npm/semver#caret-ranges-123-025-004 for excessive detail.

blimmer commented 3 years ago

Sadly, it appears that the build is broken now, so the release did not succeed. I don't have time to take a look at it for a few days, so I opened https://github.com/blimmer/cdk-datadog-integration/issues/29

blimmer commented 3 years ago

@plumdog - sorry for the delay in taking a look at this. 1.21.0 should work for you now. Thanks again for your contribution!