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

kafka_consumer: fix wrong timezone when 'TZ' env is not set and downstream is mysql #1084

Merged
merged 2 commits into from
Nov 19, 2020

Conversation

dengqee
Copy link
Contributor

@dengqee dengqee commented Nov 17, 2020

kafka_consumer: fix wrong timezone when 'TZ' env is not set and downstream is mysql

Improve robustness.

What problem does this PR solve?

*: fix wrong timezone when env TZ is not set #512 fix wrong timezone in cdc, but kafka_consumer demo still have this problem, see kafka_consumer wrong timezone when no parameter ‘--timezone’ and env ‘TZ’ is not set #1082.

What is changed and how it works?

This pr load local timezone by calling util.GetTimezone instead of use time.Local

Check List

Tests

  • Manual test (add detailed scripts or steps below)
./kafka_consumer --upstream-uri="kafka://127.0.0.1:9092/cdc-test?kafka-version=2.4.0&partition-num=6&max-message-bytes=67108864&replication-factor=1" --downstream-uri="mysql://root:123456@127.0.0.1:3306/"

--upstream-uri: the kafka sink uri of cdc
--downstream-uri: mysql sink uri

Code changes

  • Has exported function/method change
    Related changes

  • Need to cherry-pick to the release branch

Release note

kafka_consumer: fix wrong timezone when 'TZ' env is not set and downstream is mysql

@ti-srebot ti-srebot added the first-time-contributor Indicates that the PR was contributed by an external member and is a first-time contributor. label Nov 17, 2020
@amyangfei
Copy link
Contributor

Thanks for your contribution! The CI failure is related to https://github.com/pingcap/ticdc/pull/1087, it has nothing to do with this PR

@amyangfei
Copy link
Contributor

/run-kafka-tests

@amyangfei amyangfei added type/bugfix This PR fixes a bug. needs-cherry-pick-release-4.0 Should cherry pick this PR to release-4.0 branch. labels Nov 17, 2020
@amyangfei amyangfei added this to the v4.0.9 milestone Nov 17, 2020
Copy link
Contributor

@amyangfei amyangfei left a comment

Choose a reason for hiding this comment

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

LGTM

@ti-srebot ti-srebot added the status/LGT1 Indicates that a PR has LGTM 1. label Nov 18, 2020
@ti-srebot ti-srebot removed the status/LGT1 Indicates that a PR has LGTM 1. label Nov 18, 2020
@ti-srebot ti-srebot added the status/LGT2 Indicates that a PR has LGTM 2. label Nov 18, 2020
@zier-one
Copy link
Contributor

/merge

@ti-srebot ti-srebot added the status/can-merge Indicates a PR has been approved by a committer. label Nov 18, 2020
@ti-srebot
Copy link
Contributor

Your auto merge job has been accepted, waiting for:

  • 1085

@ti-srebot
Copy link
Contributor

/run-all-tests

@ti-srebot
Copy link
Contributor

@dengqee merge failed.

@amyangfei amyangfei merged commit 9ff9a78 into pingcap:master Nov 19, 2020
ti-srebot pushed a commit to ti-srebot/ticdc that referenced this pull request Nov 19, 2020
Signed-off-by: ti-srebot <ti-srebot@pingcap.com>
@ti-srebot
Copy link
Contributor

cherry pick to release-4.0 in PR #1095

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
first-time-contributor Indicates that the PR was contributed by an external member and is a first-time contributor. needs-cherry-pick-release-4.0 Should cherry pick this PR to release-4.0 branch. status/can-merge Indicates a PR has been approved by a committer. status/LGT2 Indicates that a PR has LGTM 2. type/bugfix This PR fixes a bug.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants