Repository navigation
get source-mssql CI to green - #35721
Merged
Stephane Geneix (stephane-airbyte) merged 1 commit intoMar 6, 2024
Merged
Stephane Geneix (stephane-airbyte) merged 1 commit into
Stephane Geneix (stephane-airbyte) merged 1 commit into
Conversation
|
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
Merged
Contributor
Author
This stack of pull requests is managed by Graphite. Learn more about stacking. Join Stephane Geneix (@stephane-airbyte) and the rest of your teammates on |
Octavia Squidington III (octavia-squidington-iii)
added
area/connectors
Octavia Squidington IV (octavia-squidington-iv)
requested review from
a team
February 29, 2024 08:31
Stephane Geneix (stephane-airbyte)
force-pushed
the
stephane/02-29-improve_airbyte-cdk
branch
from
February 29, 2024 08:53
d43a95d to
5fd305f
Compare
Stephane Geneix (stephane-airbyte)
force-pushed
the
stephane/02-29-get_source-mssql_CI_to_green
branch
2 times, most recently
from
February 29, 2024 17:38
4c58954 to
adef115
Compare
Octavia Squidington IV (octavia-squidington-iv)
requested a review
from a team
February 29, 2024 17:39
Stephane Geneix (stephane-airbyte)
force-pushed
the
stephane/02-29-improve_airbyte-cdk
branch
from
February 29, 2024 17:42
5fd305f to
196cd34
Compare
Stephane Geneix (stephane-airbyte)
force-pushed
the
stephane/02-29-get_source-mssql_CI_to_green
branch
3 times, most recently
from
February 29, 2024 18:09
ba5d342 to
a5d73cc
Compare
Stephane Geneix (stephane-airbyte)
requested a review
from Marius Posta (postamar)
February 29, 2024 18:10
Stephane Geneix (stephane-airbyte)
force-pushed
the
stephane/02-29-get_source-mssql_CI_to_green
branch
2 times, most recently
from
February 29, 2024 18:40
712b2e2 to
f8bbde2
Compare
Stephane Geneix (stephane-airbyte)
force-pushed
the
stephane/02-29-improve_airbyte-cdk
branch
from
February 29, 2024 20:07
196cd34 to
c77c535
Compare
Stephane Geneix (stephane-airbyte)
force-pushed
the
stephane/02-29-get_source-mssql_CI_to_green
branch
from
February 29, 2024 20:07
f8bbde2 to
76e1a66
Compare
Stephane Geneix (stephane-airbyte)
force-pushed
the
stephane/02-29-improve_airbyte-cdk
branch
2 times, most recently
from
February 29, 2024 20:37
651c9ff to
9ccfb3a
Compare
Stephane Geneix (stephane-airbyte)
force-pushed
the
stephane/02-29-get_source-mssql_CI_to_green
branch
from
February 29, 2024 20:37
76e1a66 to
0e2dd77
Compare
Stephane Geneix (stephane-airbyte)
force-pushed
the
stephane/02-29-improve_airbyte-cdk
branch
from
February 29, 2024 20:42
9ccfb3a to
d570a23
Compare
Stephane Geneix (stephane-airbyte)
force-pushed
the
stephane/02-29-get_source-mssql_CI_to_green
branch
from
February 29, 2024 20:43
0e2dd77 to
91078e0
Compare
Marius Posta (postamar)
approved these changes
Feb 29, 2024
Stephane Geneix (stephane-airbyte)
force-pushed
the
stephane/02-29-get_source-mssql_CI_to_green
branch
2 times, most recently
from
March 3, 2024 06:44
2043e3c to
135d3cc
Compare
Octavia Squidington III (octavia-squidington-iii)
added
the
CDK
Stephane Geneix (stephane-airbyte)
force-pushed
the
stephane/03-02-minor_changes_to_the_java_CDK_test_code
branch
from
March 3, 2024 06:46
c18e19d to
9bb20bd
Compare
Stephane Geneix (stephane-airbyte)
force-pushed
the
stephane/02-29-get_source-mssql_CI_to_green
branch
from
March 3, 2024 06:46
135d3cc to
d80ceb5
Compare
Octavia Squidington III (octavia-squidington-iii)
removed
the
CDK
Stephane Geneix (stephane-airbyte)
force-pushed
the
stephane/02-29-get_source-mssql_CI_to_green
branch
2 times, most recently
from
March 3, 2024 17:49
c3460c0 to
1749924
Compare
Octavia Squidington III (octavia-squidington-iii)
added
the
CDK
Contributor
There was a problem hiding this comment.
Any chance you could change your testdb methods to rethrow checked exceptions as runtime exceptions? Please. Having to propagate checked exception signatures all over the place is noisy and annoying, and not particularly useful in test harnesses.
Contributor
There was a problem hiding this comment.
Since this pattern is fairly common it might be worth introducing a retryEverySecondUntil function which takes a closure and retries as long as that closure returns a null or something
Marius Posta (postamar)
left a comment
Marius Posta (postamar)
left a comment
Contributor
There was a problem hiding this comment.
The only blocker on my end is a missing exit criterion when it comes to determining the max lsn
This was referenced Mar 4, 2024
This was referenced Mar 6, 2024
Contributor
Author
Merge activity
|
This branch was successfully deployed
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
4 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
TheAbstractMssqlSourceDatatypeTest was wrong for reals.
CdcMssqlSourceTest and CdcMssqlSslSourceTest no longer use exclusive containers.
tests that change the SQLServer agent state are now in their own test class (which needs to use an exclusive container)
Besides that, the core of the problem can be grouped in 2 issues:
a) some tests are failing to enable CDC for tables. This is due to a timing issue. We moved that logic into its own function that will try for a total of 240seconds before giving up.
b) some tests are failing when trying to read the minLsn. There is a 1sec wait implemented in the production code. Instead we introduce a busy loop that will wait for a total of 240seconds for records to appear in the CDC table before giving up. That function is implemented in test code.
Unfortunately, for both cases, we sometimes needed to wait while in the middle of a function implemented in the CDK. We introduced a few hooks in the parent PR that are implemented in this PR for the source-mssql tests, and use the functions described above