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

[Minor] Fix upstream tests failing due to #1284 #1308

Merged
merged 1 commit into from
Feb 8, 2023

Conversation

mukkachaitanya
Copy link
Member

@mukkachaitanya mukkachaitanya commented Feb 6, 2023

JdbcSourceTaskLifecycleTests were failing due to a change to API introduced as part of #1284

@@ -78,7 +78,7 @@ protected CachedConnectionProvider connectionProvider(

// Should request a connection, then should close it on stop()
EasyMock.expect(mockCachedConnectionProvider.getConnection()).andReturn(db.getConnection()).anyTimes();
mockCachedConnectionProvider.close();
mockCachedConnectionProvider.close(true);
Copy link
Member

Choose a reason for hiding this comment

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

@mukkachaitanya what does passing "true" mean?

Copy link
Member Author

Choose a reason for hiding this comment

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

True implies that the connector is stopping and we want to close the ConnectionProvider entirely. False means that the connector is closing the connection when it faces some transient errors. This was introduced as part of #1284

Copy link
Member

@ManasjyotiSharma ManasjyotiSharma left a comment

Choose a reason for hiding this comment

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

LGTM!

@mukkachaitanya mukkachaitanya merged commit d9b8fc0 into 5.5.x Feb 8, 2023
@mukkachaitanya mukkachaitanya deleted the fix-downstream-tests branch February 8, 2023 05:12
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.

None yet

2 participants