-
Notifications
You must be signed in to change notification settings - Fork 5
[Sqldef Pluging] refactor(sqldef) refactor sqldef toolRegistry download binary #36
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
Conversation
Signed-off-by: kadai0308 <[email protected]>
Signed-off-by: kadai0308 <[email protected]>
| // Currently, we create them every time the stage is executed beucause we can't pass input.Client.toolRegistry to the plugin when starting the plugin. | ||
| toolRegistry := toolRegistryPkg.NewRegistry(input.Client.ToolRegistry()) | ||
|
|
||
| // map for prevent repeatedly download sqldef |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I decided to keep it simple, just download the binary every time.
Signed-off-by: kadai0308 <[email protected]>
| // For MySQL, verify the path contains the expected binary name | ||
| if tt.dbType == config.DBTypeMySQL { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
add a TODO comment to avoid missing when supporting other types.
| lp.Errorf("Failed while getting Sqldef tool (%v)", err) | ||
| return sdk.StageStatusFailure | ||
| } | ||
| lp.Info(fmt.Sprintf("Sqldef binary downloaded: %s", sqlDefPath)) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
the path is not necessary to show on UI
…ry_test Signed-off-by: kadai0308 <[email protected]>
What this PR does
refactor sqldef toolRegistry download binary
Which issue(s) this PR fixes
Fixes #33 (comment)