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.

Sign up for GitHub

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

Jump to bottom

upgrade mssql driver #126

Merged
merged 2 commits into from
Apr 5, 2024
Merged

upgrade mssql driver #126

merged 2 commits into from
Apr 5, 2024

Conversation

laglangyue
Copy link
Contributor

@laglangyue
Copy link
Contributor Author

I know why it failed, the test code is a bit difficult, and the jdbc url is not configurable. I need some time to understand how to add the params to url

@laglangyue laglangyue force-pushed the upgrade-mssql-driver branch from 2ab7209 to f4251d2 Compare April 5, 2024 09:01
@laglangyue
Copy link
Contributor Author

@pjfanning cc

@laglangyue
Copy link
Contributor Author

image

org.apache.pekko.projection.kafka.internal.KafkaSourceProviderImplSpec

This error seems to be frequent in CI, do you know the reason? @pjfanning

@laglangyue
Copy link
Contributor Author

test KafkaSourceProviderImplSpec ok in my pc, but failed in CI

pjfanning
pjfanning approved these changes Apr 5, 2024
View reviewed changes
Copy link
Contributor

@pjfanning pjfanning left a comment

Choose a reason for hiding this comment

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

lgtm - thanks

@pjfanning pjfanning merged commit 7630c49 into apache:main Apr 5, 2024
27 of 28 checks passed
@raboof
Copy link
Member

raboof commented Apr 8, 2024

image

org.apache.pekko.projection.kafka.internal.KafkaSourceProviderImplSpec

This error seems to be frequent in CI, do you know the reason? @pjfanning

this is a known flaky test (#103). IMHO we should link to flaky tests when we encounter them (to be able to more easily see which flaky tests are in particular need of fixing), and we should re-run the tests until they are green before merging (to avoid accidentally merging with legitimate errors)

@Roiocam Roiocam mentioned this pull request Apr 8, 2024
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@pjfanning pjfanning pjfanning approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@laglangyue @raboof @pjfanning