Skip to content

Conversation

@ncordon
Copy link
Contributor

@ncordon ncordon commented Jul 3, 2023

What

This PR needs to be merged in conjunction with the latest version of the database

It makes possible to use the latest version of netty.

The issue is reproduced in a592576 and we can see how 2f0a268 solves it.

Why

A class was renamed changed between the 4.1.93.Final version of netty and 4.1.94.Final, which neo4j is intending to use. We depend on arrow, which was only updated (but not released) 2 weeks ago, so the netty dependency there needs to be an old one.

The only way to do this is to copy the file available in arrow and override it with the name change PoolThreadCache to PoolArenasCache temporarily until arrow has been released.

@ncordon ncordon force-pushed the dev-bumps-netty-version branch 3 times, most recently from 0c01aeb to 7c67284 Compare July 3, 2023 14:00
@ncordon ncordon force-pushed the dev-bumps-netty-version branch from 7c67284 to a592576 Compare July 3, 2023 14:04
}

implementation group: 'org.apache.arrow', name: 'arrow-vector', version: '12.0.1', arrowExclusions
implementation group: 'org.apache.arrow', name: 'arrow-memory-netty', version: '12.0.1', arrowExclusions
Copy link
Contributor

Choose a reason for hiding this comment

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

Don't forget to update LICENSES :)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This puzzled me cause I thought the CI was checking for those because of the work you did. I think you were right we need to add an extra step: #451

Copy link
Contributor

Choose a reason for hiding this comment

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

Hmm this is weird, because I tested it out by pushing with missing parts and it had worked then 🤔

@ncordon ncordon force-pushed the dev-bumps-netty-version branch from b2cda0a to 1383134 Compare July 4, 2023 08:29
Comment on lines 192 to +193
netty-common-4.1.93.Final.jar
netty-common-4.1.94.Final.jar
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Any hint on why do we get two versions in the LICENSES @gem-neo4j even if we are overwritting with the most recent one?

Copy link
Contributor

Choose a reason for hiding this comment

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

The LICENSE file is a mix of core and common, so I believe common doesn't have 4.1.94 yet in its dependency tree

Copy link
Contributor

Choose a reason for hiding this comment

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

Might be from test-utils actually, I see arrow is on 10.0.1 there, should that be updated?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It's not that, I've tried overriding there as well. This should heal automatically anyway when the database updates to 4.1.94

Copy link
Contributor

Choose a reason for hiding this comment

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

Yeah, should do :)

@ncordon ncordon force-pushed the dev-bumps-netty-version branch from 252d907 to 4139cd7 Compare July 4, 2023 10:33
@ncordon ncordon merged commit a312e44 into dev Jul 4, 2023
@ncordon ncordon deleted the dev-bumps-netty-version branch July 4, 2023 11:45
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants