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

Destination GCS: Fix error logs to log 'Gcs' rather than 'AWS' #17901

Merged
merged 6 commits into from
Oct 18, 2022

Conversation

zheller
Copy link
Contributor

@zheller zheller commented Oct 12, 2022

What

Fixes incorrect log message

How

Change the log copy

Recommended reading order

  1. Read it

🚨 User Impact 🚨

No

Pre-merge Checklist

n/a

Community member or Airbyter

  • Community member? Grant edit access to maintainers (instructions)
  • Secrets in the connector's spec are annotated with airbyte_secret
  • Unit & integration tests added and passing. Community members, please provide proof of success locally e.g: screenshot or copy-paste unit, integration, and acceptance test output. To run acceptance tests for a Python connector, follow instructions in the README. For java connectors run ./gradlew :airbyte-integrations:connectors:<name>:integrationTest.
  • Code reviews completed
  • Documentation updated
    • Connector's README.md
    • Connector's bootstrap.md. See description and examples
    • docs/integrations/<source or destination>/<name>.md including changelog. See changelog example
    • docs/integrations/README.md
    • airbyte-integrations/builds.md
  • PR name follows PR naming conventions

Airbyter

If this is a community PR, the Airbyte engineer reviewing this PR is responsible for the below items.

  • Create a non-forked branch based on this PR and test the below items on it
  • Build is successful
  • If new credentials are required for use in CI, add them to GSM. Instructions.
  • /test connector=connectors/<name> command is passing
  • New Connector version released on Dockerhub by running the /publish command described here
  • After the connector is published, connector added to connector index as described here
  • Seed specs have been re-generated by building the platform and committing the changes to the seed spec files, as described here
Updating a connector

Community member or Airbyter

  • Grant edit access to maintainers (instructions)
  • Secrets in the connector's spec are annotated with airbyte_secret
  • Unit & integration tests added and passing. Community members, please provide proof of success locally e.g: screenshot or copy-paste unit, integration, and acceptance test output. To run acceptance tests for a Python connector, follow instructions in the README. For java connectors run ./gradlew :airbyte-integrations:connectors:<name>:integrationTest.
  • Code reviews completed
  • Documentation updated
    • Connector's README.md
    • Connector's bootstrap.md. See description and examples
    • Changelog updated in docs/integrations/<source or destination>/<name>.md including changelog. See changelog example
  • PR name follows PR naming conventions

Airbyter

If this is a community PR, the Airbyte engineer reviewing this PR is responsible for the below items.

  • Create a non-forked branch based on this PR and test the below items on it
  • Build is successful
  • If new credentials are required for use in CI, add them to GSM. Instructions.
  • /test connector=connectors/<name> command is passing
  • New Connector version released on Dockerhub and connector version bumped by running the /publish command described here
Connector Generator
  • Issue acceptance criteria met
  • PR name follows PR naming conventions
  • If adding a new generator, add it to the list of scaffold modules being tested
  • The generator test modules (all connectors with -scaffold in their name) have been updated with the latest scaffold by running ./gradlew :airbyte-integrations:connector-templates:generator:testScaffoldTemplates then checking in your changes
  • Documentation which references the generator is updated as needed

Tests

Unit

Put your unit tests output here.

Integration

Put your integration tests output here.

Acceptance

Put your acceptance tests output here.

@zheller zheller requested a review from a team as a code owner October 12, 2022 18:35
@CLAassistant
Copy link

CLAassistant commented Oct 12, 2022

CLA assistant check
All committers have signed the CLA.

@@ -57,14 +57,14 @@ public AirbyteConnectionStatus check(final JsonNode config) {

return new AirbyteConnectionStatus().withStatus(Status.SUCCEEDED);
} catch (final AmazonS3Exception e) {
LOGGER.error("Exception attempting to access the AWS bucket: {}", e.getMessage());
LOGGER.error("Exception attempting to access the Gcs bucket: {}", e.getMessage());
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
LOGGER.error("Exception attempting to access the Gcs bucket: {}", e.getMessage());
LOGGER.error("Exception attempting to access the GCS bucket: {}", e.getMessage());

final String message = getErrorMessage(e.getErrorCode(), 0, e.getMessage(), e);
AirbyteTraceMessageUtility.emitConfigErrorTrace(e, message);
return new AirbyteConnectionStatus()
.withStatus(Status.FAILED)
.withMessage(message);
} catch (final Exception e) {
LOGGER.error("Exception attempting to access the AWS bucket: {}. Please make sure you account has all of these roles: {}", e.getMessage(),
LOGGER.error("Exception attempting to access the Gcs bucket: {}. Please make sure you account has all of these roles: {}", e.getMessage(),
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
LOGGER.error("Exception attempting to access the Gcs bucket: {}. Please make sure you account has all of these roles: {}", e.getMessage(),
LOGGER.error("Exception attempting to access the GCS bucket: {}. Please make sure you account has all of these roles: {}", e.getMessage(),

Copy link
Contributor Author

Choose a reason for hiding this comment

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

In the log message below this it is spelled Gcs, is it blocking to update that string too?

Copy link
Member

Choose a reason for hiding this comment

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

🙄 not really

Copy link
Member

@marcosmarxm marcosmarxm left a comment

Choose a reason for hiding this comment

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

@zheller can you bump the Dockerfile version and update the documentation so I can release a new version?

@zheller
Copy link
Contributor Author

zheller commented Oct 13, 2022

@marcosmarxm updated the branch to current master - did that fix?

@marcosmarxm
Copy link
Member

@marcosmarxm updated the branch to current master - did that fix?

You must update the GCS Destination Dockerfile to a new patch version and it documentation changelog too

@github-actions github-actions bot added the area/documentation Improvements or additions to documentation label Oct 18, 2022
@marcosmarxm
Copy link
Member

marcosmarxm commented Oct 18, 2022

/publish connector=connectors/destination-gcs

🕑 Publishing the following connectors:
connectors/destination-gcs
https://github.com/airbytehq/airbyte/actions/runs/3277479639


Connector Did it publish? Were definitions generated?
connectors/destination-gcs

if you have connectors that successfully published but failed definition generation, follow step 4 here ▶️

@marcosmarxm marcosmarxm merged commit d9a48c9 into airbytehq:master Oct 18, 2022
@marcosmarxm marcosmarxm changed the title Fix error logs to log 'Gcs' rather than 'AWS' Destination GCS: Fix error logs to log 'Gcs' rather than 'AWS' Oct 18, 2022
letiescanciano added a commit that referenced this pull request Oct 19, 2022
* master: (304 commits)
  Bump helm chart version reference to 0.40.27 (#18152)
  Bump helm chart version reference to 0.40.26 (#18094)
  Update deployment.yaml (#18151)
  Publishes Postgres, MySQL, MSSQL source with changes from #18041 (#18086)
  Fix minor DBT Cloud Errors. (#18147)
  Sentry Integration : Stop reporting all non system-error error types. (#18133)
  Docs: Fix backoff stategy docs (#18143)
  🐛 Destination GCS: Fix error logs to log 'Gcs' rather than 'AWS' (#17901)
  Add openAPI spec for Connector Builder Server (#17535)
  Alex/mvp UI for dbt cloud integration (#18095)
  increased timeout for sat tests (#18128)
  Bmoric/remove dep connector worker (#17977)
  `recordsRead` should be a long (#18123)
  doc_update_oath_issue_gsc (#17967)
  🎉 Source Zendesk Chat: engagements data fix infinity looping + gradlew format (#18121)
  🐛 Source Zendesk Chat: engagements data fix infinity looping (#17745)
  Custom APM Tracing (#17947)
  11679 BigQuery-Denormalized Destination: improve code coverage (#17827)
  increased timeout for sat tests (#18114)
  docs: clarify language (#18090)
  ...
jhammarstedt pushed a commit to jhammarstedt/airbyte that referenced this pull request Oct 31, 2022
…bytehq#17901)

* Fix error logs to log 'Gcs' rather than 'AWS'

* update connector version

* auto-bump connector version

Co-authored-by: marcosmarxm <marcosmarxm@gmail.com>
Co-authored-by: Octavia Squidington III <octavia-squidington-iii@users.noreply.github.com>
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/documentation Improvements or additions to documentation community connectors/destination/gcs internal team/destinations Destinations team's backlog
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants