Repository navigation
π π re-update all connectors for new CDK version and more stringent javac checks - #32125
Conversation
|
The latest updates on your projects. Learn more about Vercel for Git βοΈ 1 Ignored Deployment
|
Before Merging a Connector Pull RequestWow! What a great pull request you have here! π To merge this PR, ensure the following has been done/considered for each connector added or updated:
If the checklist is complete, but the CI check is failing,
|
|
|
||
| More details: | ||
| """; | ||
| """; |
There was a problem hiding this comment.
fixing whitespace that would be removed by the compiler but triggered a warning
|
|
||
| @Slf4j | ||
| @SuppressWarnings("try") | ||
| public class BigQueryRecordStandardConsumer extends AsyncStreamConsumer { |
There was a problem hiding this comment.
close throws Exception, hence the suppresswarnings
| */ | ||
| public class BigQueryRecordConsumer extends FailureTrackingAirbyteMessageConsumer implements AirbyteMessageConsumer { | ||
| @SuppressWarnings("try") | ||
| class BigQueryRecordConsumer extends FailureTrackingAirbyteMessageConsumer implements AirbyteMessageConsumer { |
There was a problem hiding this comment.
close() throws Exception, hence the SuppressWarnings
|
|
||
| @Override | ||
| @Deprecated | ||
| public void cleanUpStage(final String datasetId, final String stream, final List<String> stagedFiles) { |
There was a problem hiding this comment.
the overloaded cleanUpStage is deprecated,so this one needs to be annotated too
Rodi Reich Zilberman (rodireich)
left a comment
There was a problem hiding this comment.
0 warnings!
Looks good to me.
Maybe just consult with @airbytehq/destinations since a bunch of their connectors are upgraded as well.
There was a problem hiding this comment.
Can you wait until #32050 merges please? I expect it to pass CI in an hour or so, since it touches quite a few of the same connectors. It bumps them to 0.4.0 as well.
Blocking the merge is the only reason why I'm "requesting changes"
edit: incidentally, my PR fixes the destination-s3 build
|
|
||
| | Version | Date | Pull Request | Subject | | ||
| |:----------------|:-----------|:-----------------------------------------------------------|:----------------------------------------------------------------------------------------------------------------------------------------------------------------| | ||
| | 3.4.6 | 2023-11-03 | [32125](https://github.com/airbytehq/airbyte/pull/32125) | Fix compilation warnings 7 | |
|
|
||
| | Version | Date | Pull Request | Subject | | ||
| |:--------|:-----------|:-----------------------------------------------------------|:----------------------------------------------------------------------------------------------------------------------------------------------------------------| | ||
| | 2.3.6 | 2023-11-03 | [32125](https://github.com/airbytehq/airbyte/pull/32125) | fix compilation warnings | |
There was a problem hiding this comment.
nit: here and in the other changelogs, I think the significant change is rather that we're adopting version 0.4.0 of the CDK.
I'm assuming that these changelogs are read when the connector's misbehaving after an upgrade; knowing about the CDK change is probably going to be more relevant in that use case.
There was a problem hiding this comment.
I've removed myself as a blocker spinning off the connector changes in #32050 into separate PRs:
#32173
#32174
#32175
#32176
#32177
#32178
#32179
#32180
Not all have release notes, because the CDK bump is often a functional no-op.
Please feel free to do whatever suits you best with these. I set them to auto-merge before signing off.
Coverage report for source-postgres
|
β¦hecks This reverts commit 9c6ed93.
This is a no-op in term of features or bugs. This is part 2 of a series of PRs. This builds upon #32118
I'm only trying to use as many compiler warnings as possible. They're cheap and can help with a lot of bugs.
Unfortunately, until very recently, our CDK was packaged in a way that causes warnings in any package that used it. I fixed that in 0.4.0 but all projects that use local CDK will have a warning, which means we can't turn those into errors.
Since we released a new CDK (0.4.0) that exports everything properly, we now need to use it and fix the connectors that are certified