Skip to content
This repository was archived by the owner on Aug 21, 2023. It is now read-only.

support --max-idle-time argument#326

Open
dbakit wants to merge 1 commit into
pingcap:masterfrom
dbakit:add-max-idle-time-flag
Open

support --max-idle-time argument#326
dbakit wants to merge 1 commit into
pingcap:masterfrom
dbakit:add-max-idle-time-flag

Conversation

@dbakit

@dbakit dbakit commented Aug 3, 2021

Copy link
Copy Markdown

See #282

@ti-chi-bot

Copy link
Copy Markdown
Member

[REVIEW NOTIFICATION]

This pull request has not been approved.

To complete the pull request process, please ask the reviewers in the list to review by filling /cc @reviewer in the comment.
After your PR has acquired the required number of LGTMs, you can assign this pull request to the committer in the list by filling /assign @committer in the comment to help you merge this pull request.

The full list of commands accepted by this bot can be found here.

Details

Reviewer can indicate their review by submitting an approval review.
Reviewer can cancel approval by submitting a request changes review.

@CLAassistant

CLAassistant commented Aug 3, 2021

Copy link
Copy Markdown

CLA assistant check
All committers have signed the CLA.

@ti-chi-bot

Copy link
Copy Markdown
Member

Welcome @dba-kit!

It looks like this is your first PR to pingcap/dumpling 🎉.

I'm the bot to help you request reviewers, add labels and more, See available commands.

We want to make sure your contribution gets all the attention it needs!



Thank you, and welcome to pingcap/dumpling. 😃

@lichunzhu

Copy link
Copy Markdown
Contributor

@dba-kit Could you please help sign the CLA?

@lance6716

Copy link
Copy Markdown
Collaborator

this commit de1f43d is associated with another email rather than your GitHub account. You can change commit email address and fix this problem (maybe file another PR).

@dbakit

dbakit commented Aug 4, 2021

Copy link
Copy Markdown
Author

@dba-kit Could you please help sign the CLA?

done, but I find I delete a line in v4/export/config.go by mistake

_ = flags.MarkHidden(flagReadTimeout)

@lichunzhu lichunzhu left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Rest LGTM

Comment thread v4/export/config.go
flags.Bool(FlagHelp, false, "Print help message and quit")
flags.Duration(flagReadTimeout, 15*time.Minute, "I/O read timeout for db connection.")
_ = flags.MarkHidden(flagReadTimeout)
flags.Duration(flagMaxIdleTime, 600*time.Second, "max idle time for db connection.")

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

We'd better keep flagReadTimeout hidden. What's more, we'd better keep flagMaxIdleTime hidden, too. We can "un-hidden" these arguments if they are frequently used.

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

err... so those arguments still unseen to others, unless they look the source code?

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

We can add these flags to official documents.

@lichunzhu lichunzhu added the needs-cherry-pick-release-5.2 Type: Need cherry pick to release-5.2 label Oct 12, 2021
@lichunzhu

Copy link
Copy Markdown
Contributor

@dba-kit hello, Dumpling has been moved to https://github.com/pingcap/tidb/tree/master/dumpling now. Could you please help move this PR to pingcap/tidb?

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants