sidorares / node-mysql2

:zap: fast mysqljs/mysql compatible mysql driver for node.js
https://sidorares.github.io/node-mysql2/
MIT License
4.07k stars 618 forks source link

Releases? #1631

Closed ErisDS closed 1 year ago

ErisDS commented 2 years ago

Hi there,

My team and I (Ghost) are trying to understand why this repo doesn't get releases?

There are regular updates to the repo but noone is benefitting from the changes - the last release was back in November 2021. At which point the releases had been every couple of weeks.

We have users specifically waiting on https://github.com/sidorares/node-mysql2/pull/1438, which was merged 10days after the last release, now almost a year ago.

Is there a problem with the release process? Something we could possibly help with? cc @daniellockyer

We really appreciate all of the hard work going on here - we would love to make use of it!

sidorares commented 2 years ago

Mainly because I'm very busy and publishing is currently not fully automatic. I'm going to try to find some time and configure release-please or something similar over the weekend

sidorares commented 2 years ago

Re help - any release workflow automation would be appreciated. Also one blocker is coverage check that is failing on PRS where source branch outside of this repo. Need to fix the issue or remove the check

ErisDS commented 2 years ago

Hey @sidorares, I appreciate how difficult these things are when they're not automated. I believe @daniellockyer fixed a similar issue with PR coverage checks on our repos so hopefully he can take a look at that next week.

He's also a dab hand at workflow automation and I'm sure he can help more.

daniellockyer commented 2 years ago

Hey @sidorares 👋🏻 Apologies for stepping in much later. I'd be more than happy to help given we also fixed up the node-sqlite3 release process too 🙂

I see you've pushed a commit for the coverage check permissions. Shall I take a look into the release-please GHA?

sidorares commented 2 years ago

Thanks @daniellockyer!

yes, I was thinking to add release-please

I'll describe current manual release process and we can discuss what automated would look like:

1) clone master locally, go through PR/commits and extract what I think needs to be communicated into https://github.com/sidorares/node-mysql2/blob/master/Changelog.md 2) commit changelog as vX.Y.Z changes, run npm version patch ( or minor/major based on the changes ) 3) push changelog commit to master + commit and tag generated by npm version 4) npm publish

Sometimes where I feel changes are too big there is an -rc version publish followed by few days of waiting.

I still prefer manual changelog, my ideal workflow would be this:

Does not have to be that fancy though. NPM_TOKEN secret is already set on this repo

daniellockyer commented 2 years ago

@sidorares This should all be doable AFAICT 🙂

My only question is that release-please uses conventional commits to determine the bump type. This repo doesn't seem to use that right now, so the suggested version might be off.

Is that something you'd be open to using?

daniellockyer commented 2 years ago

In the mean time, I've opened https://github.com/sidorares/node-mysql2/pull/1647 🙂

daniellockyer commented 2 years ago

@sidorares Is there anything else I can do to help with a release? 🙂

sidorares commented 2 years ago

hey @daniellockyer looks like release-please action is working and doing its job, I just need to check what I want to do with the first release. Looks like if I merge https://github.com/sidorares/node-mysql2/pull/1648 it'll publish v2.3.4 do you know how do I force major release? Should I push Release-As commit? https://github.com/googleapis/release-please#how-do-i-change-the-version-number

Sorry for the silly questions, the docs seems to answer them but don't want to do anything dumb acciedentally and then rush to fix that

daniellockyer commented 2 years ago

I believe you can do either that, or temporarily edit the workflow to have release-as: '3.0.0' as per the docs.

CleanShot 2022-10-18 at 11 10 51@2x
silverbullettruck2001 commented 2 years ago

@sidorares is there any chance of the release getting published before the end of the month?

stebet commented 2 years ago

Great to see this collaboration from @ErisDS and @daniellockyer from Ghost here. I'd love to see a release for this as well since I'm depending in MySQL for Azure running as the backend for my blog and I've been waiting for this fix for some time now.

I'm going to throw in a sponsorship for @sidorares as well. I have a small taste of how hard and time-consuming maintaining an OSS project can be especially when you have other obligations as well, so every little bit helps :)

Looking forward to seeing the next release!

sidorares commented 2 years ago

Thanks for your support @stebet

@silverbullettruck2001 was going to write "yes" but just realised its already Nov 1st. I really want to have a test release-please driven release first, I'll prepare a manual changelog and push "release-as: 3.0.0-rc0" some time later tomorrow. Ifter that hopefully going to be just a matter of merging a PR generated by release-please action moving forward

Sorry for keeping delaying this, unfortunately lots of other commitments preventing for fully focusing on the work here. Thanks again everyone who helped

silverbullettruck2001 commented 2 years ago

@sidorares That is great news! Thanks again for you continued dedication to this. I definitely understand why you are prioritizing release-please to make this easier for you and the community. Please keep up the good work!

aysark commented 1 year ago

Any update on release? Thanks.

sidorares commented 1 year ago

@ErisDS @aysark @silverbullettruck2001 published v3.0.0-rc.1 to npm

@daniellockyer for some reason publish step failed in the workflow, had to publish manually: https://github.com/sidorares/node-mysql2/actions/runs/3404535753/jobs/5661839917

rafipiccolo commented 1 year ago

Sorry to complain, but I believe that a release candidate "v3.0.0-rc.1" should not be noted as latest on npm. https://www.npmjs.com/package/mysql2?activeTab=versions

ErisDS commented 1 year ago

@sidorares Thank you so much 🎉 just a headsup @daniellockyer is taking a well-earned screen break this week, but I'll get him to help with this asap next week.

@rafipiccolo that's how npm works by default. Although it can be changed later using npm dist-tag, it is super fiddly and really not what I'd want to ask Sidores's to spend his limited and extremely valuable time on right now.

rafipiccolo commented 1 year ago

ok, i was only reporting. i'll adapt my cicd in any case.

sidorares commented 1 year ago

Sorry to complain, but I believe that a release candidate "v3.0.0-rc.1" should not be noted as latest on npm. https://www.npmjs.com/package/mysql2?activeTab=versions

good point @rafipiccolo , you are right

changed v2.3.3 tag back to latest and tagged v3.0.0-rc.1 as next

> npm dist-tag add mysql2@3.0.0-rc.1 next
> npm dist-tag add mysql2@2.3.3 latest
sidorares commented 1 year ago

no rush @ErisDS, I can continue to publish manually, it's already easier from now on. We'll get everything running smoothly eventually

sidorares commented 1 year ago

@rafipiccolo that's how npm works by default. Although it can be changed later using npm dist-tag, it is super fiddly and really not what I'd want to ask Sidores's to spend his limited and extremely valuable time on right now.

TIL ( I knew about tags but now I know how to set them at publish time and how to change later )

ErisDS commented 1 year ago

Sorry for being defensive, I just know how it feels when you finally ship the thing everyone asks for & the replies are all "And another thing..." 😬

Really appreciate you getting this out. I know @daniellockyer is gonna be stoked, and there's a tonne of people in the Ghost community who are gonna be over the moon to be able to upgrade as well ❤️

silverbullettruck2001 commented 1 year ago

@sidorares Congrats! 🎉 Thank you SOOO much for getting this out! I will be interested to see you get the automation completed for the next release so that this process can be much easier for you! 👍 😃

silverbullettruck2001 commented 1 year ago

@sidorares I am sorry to report that I think there is something odd going on with the RC that was generated. When I do an npm i to install 3.0.0-rc.1 it installs, but the when I try to specify authPlugins in the ConnectionOptions parameter it no longer has authPlugins as property. When I look at the ConnectionOptions interface in the index.d.ts file that it installed locally it doesn't match what is currently in source control. In the screenshot below you can see what is installed locally on the left hand side and what is in source control on the right hand side. Can this be fixed?

image

MasterOdin commented 1 year ago

@silverbullettruck2001 I just installed the rc and I'm getting the right types for it. Looking at https://unpkg.com/browse/mysql2@3.0.0-rc.1/index.d.ts and that looks right as well. Are you pulling in a types file from somewhere else or an old version or something?

silverbullettruck2001 commented 1 year ago

@MasterOdin I was able to fix this so that I am getting the complete types now. However, I am having issues getting the cleartext part to work. Here is an example of what I am doing. Is this how it should be coded to specify the cleartext plugin? When I use this I am still getting Access Denied but if I log in using MySql Workbench using the cleartext plugin the same credentials work, so I am thinking I have something incorrect. Could you provide an example of how you have this working?

// create the connection to database
const connection = mysql.createConnection({
    host: 'localhost',
    user: 'root',
    password: 'pass',
    database: 'test',
    authPlugins: {
        mysql_clear_password: () => () => {
            return Buffer.from(node.password + '\0')
        }
    },
});

// simple query
connection.query('SELECT 1', (err, results, fields) => {
    console.log(err, results);
});
stebet commented 1 year ago

On that note, when can we expect Ghost to update this dependency @ErisDS @daniellockyer ?

stebet commented 1 year ago

On that note, when can we expect Ghost to update this dependency @ErisDS @daniellockyer ?

Ping :)

ErisDS commented 1 year ago

Ghost's dependency updates are largely automated, but won't pick up the RC. We will look at doing a manual update in the new year if there's no movement on an official v3 & it seems the RC is stable 🙂

sidorares commented 1 year ago

@daniellockyer I think the problem was on my side, I incorrectly configured npm token, though error messages were not very helpful in debugging that. Looks like after few forced release-please runs its all working OK now and we should see a release pushed to npm after every merged PR

Not sure how to deal with dependabot PRs, I'd rather batch them to avoid noise in releses. Do I just close automatic release-please PR? Or keep it open while I merge PRs and only merge release PR when I'm ready to push? I guess I'll try that next time

Closing this for now, thanks everyone who helped with actions code, advises, friendly pings and any other support 🤟

daniellockyer commented 1 year ago

@sidorares Apologies, I missed the earlier ping! This is great - thanks for getting the release out 🙂

daniellockyer commented 1 year ago

It's really great to see so many commits + releases in this project again 😀