chrisguttandin / dynamo-db-local

A wrapper around Amazon's DynamoDB Local to start and stop it from Node.js.
MIT License
13 stars 5 forks source link

Improve logging & possibly fix issues w/ Node.js 17+ #112

Closed o-alexandrov closed 1 year ago

o-alexandrov commented 1 year ago

DynamoDB doesn't spin up for me w/ Node.js 17+, unless stdio: 'inherit' is set.

But, primarily, this PR should improve logging of what is being spawned. Users should start seeing similar to the following in their CLI:

Screenshot 2022-12-17 at 2 11 50 AM
o-alexandrov commented 1 year ago

@chrisguttandin thank you for merging this and kindly adjusting the tests in d6fb0e6b163059a1a01d5d627828dfb488eb1e1f. I apologize for disturbing you, could you please let me know when do you plan to publish the change?

chrisguttandin commented 1 year ago

Hi @o-alexandrov, sorry for the delay.

I usually use a bot to publish new versions. It was already configured to drop support for Node.js v12 which means it would publish a major version since that's possibly a breaking change. I didn't want that to happen while everyone is on holidays and that's why I paused it for now.

Long story short, I just published v4.2.0 which contains your changes. Thanks again for your contribution. Likely there will be a v5 release later this week which drops support for Node.js v12.

chrisguttandin commented 1 year ago

Hi @o-alexandrov, I realized the hard way that this was a breaking change. I therefore pushed another change which sets stdio to 'pipe' which is the default value. But it is possible to override this by providing a different value. I hope this works for you, too.

https://github.com/chrisguttandin/dynamo-db-local/commit/a4e0a92b8258952656d51ac302f54d6db61064fb

o-alexandrov commented 1 year ago

Yes, nice, thank you for letting to override and thoughtfully keeping the default behavior. Apologies for introducing a breaking change, I didn't expect that

chrisguttandin commented 1 year ago

No worries, I didn't see this coming, too.