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

removed unused code #18788

Merged
merged 9 commits into from
Nov 3, 2022
Merged

removed unused code #18788

merged 9 commits into from
Nov 3, 2022

Conversation

colesnodgrass
Copy link
Member

What

  • in commons
    • remove unused code
    • replace old code patterns with newer java functionality
    • ran ./gradlew format

@github-actions
Copy link
Contributor

github-actions bot commented Nov 1, 2022

NOTE ⚠️ Changes in this PR affect the following connectors. Make sure to do the following as needed:

  • Run integration tests
  • Bump connector version
  • Add changelog
  • Publish the new version

⚠ Sources (24)

Connector Version Changelog Publish
source-alloydb 1.0.17
source-alloydb-strict-encrypt 1.0.17
(not in seed)
source-bigquery 0.2.2
source-clickhouse 0.1.14
source-clickhouse-strict-encrypt 0.1.14
(not in seed)
source-cockroachdb 0.1.18
source-cockroachdb-strict-encrypt 0.1.18
(not in seed)
source-db2 0.1.16
source-db2-strict-encrypt 0.1.16
(not in seed)
source-jdbc 0.3.5
(doc not found)

(not in seed)
source-mongodb-strict-encrypt 0.1.19
(doc not found)

(not in seed)
source-mongodb-v2 0.1.19
source-mssql 0.4.24
source-mssql-strict-encrypt 0.4.24
(not in seed)
source-mysql 1.0.10
source-mysql-strict-encrypt 1.0.10
(not in seed)
source-oracle 0.3.21
source-oracle-strict-encrypt 0.3.21
(not in seed)
source-postgres 1.0.22
source-postgres-strict-encrypt 1.0.22
(not in seed)
source-redshift 0.3.14
source-scaffold-java-jdbc 0.1.0
(doc not found)

(not in seed)
source-snowflake 0.1.24
source-tidb 0.2.1

❌ Destinations (43)

Connector Version Changelog Publish
destination-aws-datalake 0.1.1
destination-azure-blob-storage 0.1.6
(doc not found)
destination-bigquery 1.2.5
destination-bigquery-denormalized 1.2.5
(doc not found)
destination-cassandra 0.1.4
(changelog missing)
destination-clickhouse 0.2.0
destination-clickhouse-strict-encrypt 0.2.0
(not in seed)
destination-csv 0.2.10
(doc not found)
destination-databricks 0.3.1
destination-dev-null 0.2.7
(doc not found)

(not in seed)
destination-dynamodb 0.1.5
destination-e2e-test 0.2.4
destination-elasticsearch 0.1.6
destination-elasticsearch-strict-encrypt 0.1.6
(not in seed)
destination-gcs 0.2.12
destination-jdbc 0.3.14
(doc not found)

(not in seed)
destination-kafka 0.1.10
(changelog missing)
destination-keen 0.2.4
(changelog missing)
destination-kinesis 0.1.5
(changelog missing)
destination-local-json 0.2.11
destination-mariadb-columnstore 0.1.7
destination-mongodb 0.1.8
destination-mongodb-strict-encrypt 0.1.8
(not in seed)
destination-mqtt 0.1.3
destination-mssql 0.1.22
destination-mssql-strict-encrypt 0.1.22
(not in seed)
destination-mysql 0.1.20
destination-mysql-strict-encrypt 0.1.21
(mismatch: 0.1.20)

(not in seed)
destination-oracle 0.1.19
destination-oracle-strict-encrypt 0.1.19
(not in seed)
destination-postgres 0.3.26
destination-postgres-strict-encrypt 0.3.26
(not in seed)
destination-pubsub 0.1.6
(changelog missing)
destination-pulsar 0.1.3
(changelog missing)
destination-r2 0.1.0
destination-redis 0.1.4
destination-redshift 0.3.51
destination-rockset 0.1.4
(changelog missing)
destination-s3 0.3.17
destination-scylla 0.1.3
destination-snowflake 0.4.38
destination-tidb 0.1.0
destination-yugabytedb 0.1.0

Notes

Category Status Actionable Item
Version
mismatch
The version of the connector is different from its normal variant. Please bump the version of the connector.

doc not found
The connector does not seem to have a documentation file. This can be normal (e.g. basic connector like source-jdbc is not published or documented). Please double-check to make sure that it is not a bug.
Changelog
doc not found
The connector does not seem to have a documentation file. This can be normal (e.g. basic connector like source-jdbc is not published or documented). Please double-check to make sure that it is not a bug.

changelog missing
There is no chnagelog for the current version of the connector. If you are the author of the current version, please add a changelog.
Publish
not in seed
The connector is not in the seed file (e.g. source_definitions.yaml), so its publication status cannot be checked. This can be normal (e.g. some connectors are cloud-specific, and only listed in the cloud seed file). Please double-check to make sure that it is not a bug.

diff seed version
The connector exists in the seed file, but the latest version is not listed there. This usually means that the latest version is not published. Please use the /publish command to publish the latest version.

@colesnodgrass colesnodgrass temporarily deployed to more-secrets November 1, 2022 17:16 Inactive
@colesnodgrass colesnodgrass temporarily deployed to more-secrets November 1, 2022 17:49 Inactive
@colesnodgrass colesnodgrass temporarily deployed to more-secrets November 1, 2022 19:22 Inactive
@colesnodgrass colesnodgrass marked this pull request as ready for review November 1, 2022 20:03
@colesnodgrass colesnodgrass requested a review from a team as a code owner November 1, 2022 20:03
Copy link
Contributor

@benmoriceau benmoriceau left a comment

Choose a reason for hiding this comment

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

Can we check that cloud won't be broken by that change?

@colesnodgrass colesnodgrass temporarily deployed to more-secrets November 1, 2022 20:07 Inactive
@colesnodgrass
Copy link
Member Author

@benmoriceau that's what I'm doing right now

@colesnodgrass colesnodgrass temporarily deployed to more-secrets November 1, 2022 20:10 Inactive
@colesnodgrass colesnodgrass temporarily deployed to more-secrets November 1, 2022 21:33 Inactive
@colesnodgrass colesnodgrass temporarily deployed to more-secrets November 1, 2022 22:12 Inactive
@colesnodgrass colesnodgrass temporarily deployed to more-secrets November 2, 2022 17:55 Inactive
@colesnodgrass
Copy link
Member Author

I was able to run the cloud locally without any issues.

@@ -58,7 +58,8 @@ public class JsonSchemas {
* @param jsonNode - a json object with children that contain types.
*/
public static void mutateTypeToArrayStandard(final JsonNode jsonNode) {
if (jsonNode.get(JSON_SCHEMA_TYPE_KEY) != null && !jsonNode.get(JSON_SCHEMA_TYPE_KEY).isArray()) {
if (jsonNode.get(JSON_SCHEMA_TYPE_KEY) != null && !jsonNode.get(JSON_SCHEMA_TYPE_KEY)
Copy link
Contributor

Choose a reason for hiding this comment

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

@colesnodgrass i think your IDE is using a shorter line length than our standard. can you make sure it is set to 150 (docs)

Copy link
Member Author

Choose a reason for hiding this comment

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

We actually have two different settings. In the docs we use 150 but in the style-sheet we set comment lines to 100. Looks like for some reason the 100 length was also being used for non-comment lines.

We should probably increase the comment-line length to match the 150 value.

* This is a temporary class that is being provided to ensure that resources created by each
* application are properly closed on shutdown. This logic will no longer be necessary once an
* application framework is introduced to the project that can provide object lifecycle management.
*/
public class CloseableShutdownHook {

private static final Logger LOGGER = LoggerFactory.getLogger(CloseableShutdownHook.class);
private static final Logger log = LoggerFactory.getLogger(CloseableShutdownHook.class);
Copy link
Contributor

Choose a reason for hiding this comment

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

why switch from all caps to all down case?

Copy link
Member Author

Choose a reason for hiding this comment

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

The @SLF4j annotation generates a lower case log variable:

Example:
  @Slf4j
  public class LogExample {
  }
  
will generate:
  public class LogExample {
      private static final org.slf4j.Logger log = org.slf4j.LoggerFactory.getLogger(LogExample.class);
  }  

And I've been stripping out this annotation and replacing it with non-generated log code. We presently have a mix of log and LOGGER and should standardize on one and update everything to match.

Copy link
Contributor

@cgardens cgardens left a comment

Choose a reason for hiding this comment

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

the deletions look good. can we fix the short line break before merging, please?

@colesnodgrass colesnodgrass temporarily deployed to more-secrets November 3, 2022 16:18 Inactive
@github-actions github-actions bot added the area/worker Related to worker label Nov 3, 2022
Copy link
Contributor

@jdpgrailsdev jdpgrailsdev left a comment

Choose a reason for hiding this comment

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

:shipit:

@colesnodgrass colesnodgrass merged commit 9231e3e into master Nov 3, 2022
@colesnodgrass colesnodgrass deleted the cole/remove-unused-code branch November 3, 2022 17:32
final List<AirbyteMessage> destinationOutput = runSync(config, messages, catalog,
runNormalization);

Collections.reverse(messages);
Copy link
Contributor

@Leibnizhu Leibnizhu Nov 4, 2022

Choose a reason for hiding this comment

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

Collections.reverse changes the order of messages.

In previous version of code, MoreLists.reversed generates a new List with reversed messages, but the order of origin messages don't change.

this code change may cause some integration/acceptance test fail

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/connectors Connector related issues area/platform issues related to the platform area/protocol area/worker Related to worker connectors/source/relational-db
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

6 participants