-
Notifications
You must be signed in to change notification settings - Fork 37
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
Add tests that access AWS #208
Conversation
- Test authentication via AssumeRole - Test `streamChanges: true` both with and without `skipInitialSnapshotTransfer`
…redentials - Get temporary credentials from the GitHub workflow, and forward those credentials to the Spark nodes - Remove AssumeRoleTest, which cannot be executed anymore
…ided on the source table
…state, otherwise it never terminates, and properly clean-up the handlers.
- uses: actions/setup-java@v4 | ||
with: | ||
distribution: temurin | ||
java-version: 8 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I thought we compile with jdk 11
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
wait, ok, it was blocked due to spark 2.x
ok, so maybe we can switch to jdk 11 with spark 3.x
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is not really important to be support Java 8, but if we can, why not do it? Using Java 8 in the CI ensures that we at least support Java 8. On the other hand, that could also prevent us from detecting possible incompatibilities with higher versions of the JVM…
But anyway, that should be a separate issue/PR :)
Alternative to #205. Fixes #113.
The first commit (4945055) is taken from #205.
The second commit (127429d) changes the authentication workflow to rely on GitHub as an OpenID Connect provider (instead of using AssumeRole with static AWS credentials, which can not expose in the CI, even as GitHub secrets), and removes the AssumeRole authentication test.
The remaining commits fix issues that I discovered while working on this, and could be extracted out to separate PRs if needed:
streamChanges
is enabled but no static credentials were provided. We used to fail with aNullPointerException
. We now fallback to the default AWS credentials provider.