Skip to content

Conversation

@tisonkun
Copy link
Member

No description provided.

ByteBuffer duplicate = si.request.duplicate();
duplicate.rewind();
ByteBufferInputStream.byteBuffer2Record(duplicate, createRequest);
CreateRequest createRequest = si.readRequestRecord(CreateRequest::new);
Copy link
Member Author

Choose a reason for hiding this comment

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

This may cause failures on early ZK versions and we need a testing compatible layer.

Let the CI verify if it's the case.

Copy link
Member Author

Choose a reason for hiding this comment

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

OK. It fails.

Let me think if we should drop support <= 3.7 or add a test compatible switch.

cc @kezhuw @eolivelli @cammckenzie @Randgalt

@eolivelli
Copy link
Contributor

I think that it is fine to drop support for 3.7 in those tests

@tisonkun
Copy link
Member Author

tisonkun commented Feb 1, 2024

Make sense. I understand it as introducing a new test tag said zk38OrLater to turn on related tests only when zk 3.8+ as dep.

@tisonkun
Copy link
Member Author

[ERROR] Tests run: 3, Failures: 0, Errors: 3, Skipped: 0, Time elapsed: 0.44 s <<< FAILURE! - in org.apache.curator.framework.imps.TestTransactionsOld
[ERROR] org.apache.curator.framework.imps.TestTransactionsOld.testWithCompression  Time elapsed: 0.144 s  <<< ERROR!
java.lang.NoSuchMethodError: 'com.google.common.base.Predicate org.apache.curator.framework.api.transaction.CuratorTransactionResult.ofTypeAndPath(org.apache.curator.framework.api.transaction.OperationType, java.lang.String)'
	at org.apache.curator.framework.imps.TestTransactionsOld.testWithCompression(TestTransactionsOld.java:165)

It seems because we relocate these methods in curator-framework but not for the test-jar. I don't find a way to relocate the test jar now ..

@tisonkun
Copy link
Member Author

tisonkun commented Feb 14, 2024

I create a monkey patch to work around it - c44e798

Signed-off-by: tison <[email protected]>
Copy link
Contributor

@eolivelli eolivelli left a comment

Choose a reason for hiding this comment

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

+1

Signed-off-by: tison <[email protected]>
@tisonkun
Copy link
Member Author

tisonkun commented Feb 14, 2024

@eolivelli Thanks for your review. I make this patch work now. Let me create the ticket and a few follow-ups to improve the case (It now takes 2 hours to complete, I believe we can drop a few versions, but better with a few separate tickets).

@tisonkun tisonkun changed the title CURATOR-XXX. Upgrade ZooKeeper version to 3.9 CURATOR-699. Upgrade ZooKeeper version to 3.9 Feb 15, 2024
@tisonkun tisonkun merged commit 972fffa into apache:master Feb 15, 2024
@tisonkun tisonkun deleted the zk39 branch February 15, 2024 02:30
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.

2 participants