Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Use newer version of mysql-binlog-connector-java #52

Merged
merged 2 commits into from
Oct 26, 2020

Conversation

Chrizpy
Copy link
Contributor

@Chrizpy Chrizpy commented Oct 21, 2020

Close #49

The original author is no longer maintaining the one we are using.

@walro
Copy link
Contributor

walro commented Oct 21, 2020

I was intrigued by the fact that the fork was 6 commits after "upstream", here's the missing stuff:

osheroff/mysql-binlog-connector-java@master...shyiko:master

Wonder if those things have been solved 🤔

Edit: osheroff/mysql-binlog-connector-java@master...shyiko:c54b4c1 these are the "relevant" changes between the two repos

@walro
Copy link
Contributor

walro commented Oct 21, 2020

Wonder if those things have been solved 🤔

As far as I can tell, out of the 6 commits, only 3 of them changes anything substantial, these are:

The last one of these was fixed in the fork too by osheroff/mysql-binlog-connector-java@88de1ed

However, I don't see the other two being addressed in the fork. They both seem to be a part of solving shyiko/mysql-binlog-connector-java#321

Not sure if that's an issue for us, but we could see if the fork could pull those changes in for some peace of mind.

Edit: A bit shorter diff version for the stuff that matters: osheroff/mysql-binlog-connector-java@master...shyiko:c54b4c1

@Chrizpy
Copy link
Contributor Author

Chrizpy commented Oct 22, 2020

I was intrigued by the fact that the fork was 6 commits after "upstream", here's the missing stuff:

osheroff/mysql-binlog-connector-java@master...shyiko:master

Wonder if those things have been solved 🤔

I was really confused by this comment for a while, I thought you meant that the fork was 6 ahead of the latest commit on "upstream". But you probably meant that it is 6 commits behind 😄 .

That makes a lot more sense. Seeing as we need (osheroff/mysql-binlog-connector-java@a6a3af7) for the last test case in #53 to become green, supposedly. Should we switch back to using the old repository then and wait on this fork to progress?

@walro
Copy link
Contributor

walro commented Oct 22, 2020

But you probably meant that it is 6 commits behind

Yes, I should have typed "behind" instead of "after" :)

@walro
Copy link
Contributor

walro commented Oct 22, 2020

Should we switch back to using the old repository then and wait on this fork to progress?

Not sure, we need to switch sooner or later. What I was thinking is that we could submit a PR (if it's resonable) with the fix that's missing in the fork.

@walro

This comment has been minimized.

Related to: #49

The original author is no longer maintaining the one we are using.
Removing the old .jar file since we don't use it anymore.
@walro
Copy link
Contributor

walro commented Oct 23, 2020

Since we don't use keepAlive in our "Ecco clients", I am pretty sure that we won't be affected by shyiko/mysql-binlog-connector-java#321. However, as we allow other users of Ecco to use keepAlive, I believe it would be cool if we could get the changes merged into the forked repo.

I will see what can be done, for now I believe we can merge this just to get MySQL 8.0 support going.

Copy link
Contributor

@walro walro left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

See comment in #52 (comment)

@Chrizpy Chrizpy merged commit d7733f0 into master Oct 26, 2020
@Chrizpy Chrizpy deleted the update-binlog-connector branch October 26, 2020 07:24
walro added a commit that referenced this pull request Oct 26, 2020
As of #52 we have green tests for MySQL 8.0, so let's ensure we keep them green :)

Related to #41
@Chrizpy
Copy link
Contributor Author

Chrizpy commented Oct 26, 2020

Closing this issue: #41 since this PR adds MySQL 8.0 support.

@Chrizpy Chrizpy mentioned this pull request Oct 26, 2020
@Chrizpy
Copy link
Contributor Author

Chrizpy commented Nov 2, 2020

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

mysql-binlog-connector-java is no longer maintained
2 participants