Skip to content

Small fixes in move-tables config initialization#1712

Merged
danieljoos merged 1 commit into
feature-move-tablesfrom
danieljoos-move-table-config-fixes
Jun 17, 2026
Merged

Small fixes in move-tables config initialization#1712
danieljoos merged 1 commit into
feature-move-tablesfrom
danieljoos-move-table-config-fixes

Conversation

@danieljoos

Copy link
Copy Markdown
Contributor

A Pull Request should be associated with an Issue.

We wish to have discussions in Issues. A single issue may be targeted by multiple PRs.
If you're offering a new feature or fixing anything, we'd like to know beforehand in Issues,
and potentially we'll be able to point development in a particular direction.

Related issue: #1681

Further notes in https://github.com/github/gh-ost/blob/master/.github/CONTRIBUTING.md
Thank you! We are open to PRs, but please understand if for technical reasons we are unable to accept each and any PR

Description

This PR fixes the credential loading for move-tables mode. It only uses the --target-user and --target-password if not empty. Otherwise it re-uses the same credentials as for the source connection.

It also fixes the lock database connection, using the target database instead of the source database name. These can differ in move-tables mode.

Copilot AI review requested due to automatic review settings June 17, 2026 13:12

Copilot AI 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.

Pull request overview

This pull request fixes move-tables mode initialization so the target connection credentials and migration lock DB selection behave correctly when the target DB/credentials differ from the source, aligning runtime behavior with the move-tables connection model described in issue #1681.

Changes:

  • Derive move-tables connection credentials from the source/inspector credentials, and only override them when --target-user/--target-password are explicitly provided.
  • Use the target database name when opening the dedicated DB handle used to acquire the migration user-level lock.
  • Remove the earlier CLI-time defaulting of TargetUser/TargetPass, centralizing the behavior in ApplyCredentials().
Show a summary per file
File Description
go/logic/applier.go Uses GetTargetDatabaseName() for migration lock name/DB URI so lock acquisition works when target DB differs.
go/cmd/gh-ost/main.go Removes move-tables credential defaulting at flag-parsing time; relies on centralized credential application.
go/base/context.go Applies move-tables credential overrides only when non-empty; otherwise reuses inspector/source credentials.

Copilot's findings

Tip

Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

  • Files reviewed: 3/3 changed files
  • Comments generated: 1

Comment thread go/base/context.go
Comment on lines +986 to +990
if mctx.MoveTables.TargetUser != "" {
// Override
mctx.MoveTables.ConnectionConfig.User = mctx.MoveTables.TargetUser
}
if mctx.MoveTables.TargetPass != "" {
@danieljoos danieljoos merged commit fa9b2bc into feature-move-tables Jun 17, 2026
7 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants