search

mysqljs / mysql

A pure node.js JavaScript Client implementing the MySQL protocol.
MIT License
18.31k stars 2.53k forks source link

feat: add dump function for current connections count #2562

Closed fengmk2 closed 1 year ago

sidorares commented 1 year ago

I'm probably +1 to expose this information somehow At the same time I'd prefer to minimise surface api, since queryingConnectionsCount can be computed from other 3 maybe not worth adding

Also re naming: instead dumpConnectionsCount - maybe something like connectionsInformation or getConnectionsInformation. Also maybe idleConnectionsCount instead of freeConnectionsCount - need to check if related information is exposed under similar name

sidorares commented 1 year ago

looks like CI config need some refresh, potentially dropping node 0.10 and moving appveyor to actions windows runner

fengmk2 commented 1 year ago

cc @qile222

dougwilson commented 1 year ago

Well, we don't want to remove node.js versions from the CI that are supported by the current version. Usually removing support is a semver major action. Taking a look, the CI appveypr seem to be fine, just the new code in the PR needs to be adjusted to actually work in Node.js 0.10 is all (I suspect it is the use of deepStrictEqual in the tests as I don't think thay existed then). And I will update gh actions runner host for those.

As for the change, it seems fine, though whay about just calling it getConnnectionCounts() or such?

dougwilson commented 1 year ago

Also, I think maybe queryingConnectionsCount is going to.be confusing, since the math for thay number is not the number of connections making a query, but just like the connection that are checked out from the pool, even if they are just idle.