feat(cli): Use SourceClient and DestinationClient directly#2165
feat(cli): Use SourceClient and DestinationClient directly#2165yevgenypats merged 4 commits intomainfrom
Conversation
PluginManager abstraction is not needed anymore. makes less prop drilling and less code. see cloudquery/plugin-sdk#208
|
The versions should go prob also to the SDK but we can do that later if needed. The most important thing was to get rid off the heavy pluginmanager abstraction anyway. |
cli/cmd/sync.go
Outdated
| destPlugin.Close() | ||
| for _, destClient := range destClients { | ||
| if destClient != nil { | ||
| destClient.Close() |
There was a problem hiding this comment.
Not directly related to the work in this PR, but technically this Close returns an error that we're ignoring, we may want to at least log it
There was a problem hiding this comment.
Good catch. Print is good - log is already done on the sdk side (though I should add some more there)
| totalResources, failedWrites, sourcePlugin.Errors(), sourcePlugin.Warnings()) | ||
| if sourcePlugin.Errors() > 0 || sourcePlugin.Warnings() > 0 || failedWrites > 0 { | ||
| fmt.Println("Please check the logs for more details on errors/warnings.") | ||
| } |
There was a problem hiding this comment.
Why are these messages being removed, were they moved elsewhere?
There was a problem hiding this comment.
I need to open a follow-up PR but I want us to stop parsing logs and the plugins return number of error and or warnings as they have this information anyway, this will make also consistent experience between debug and release (which I know was quite weird before)
There was a problem hiding this comment.
Nice, yeah that would be great
There was a problem hiding this comment.
Maybe keep these lines in there for now though, we can remove them in the follow-up?
cli/cmd/sync.go
Outdated
| } | ||
| defer sourcePlugin.Close() | ||
| sourceClient := sourcePlugin.GetClient() | ||
| defer sourceClient.Close() |
There was a problem hiding this comment.
I think this also ignores any error we may get from Close
|
Much simpler 👍 I left a few comments unrelated to this PR really, so feel free to ignore, but we should probably address the error handling there at some point. The only real question for me is around the print lines being removed |
Co-authored-by: Herman Schaaf <hermanschaaf@gmail.com>
🤖 I have created a release *beep* *boop* --- ## [1.3.0-pre.1](cli-v1.2.1-pre.1...cli-v1.3.0-pre.1) (2022-09-30) ### Features * **cli:** Use SourceClient and DestinationClient directly ([#2165](#2165)) ([e594e61](e594e61)) ### Bug Fixes * ProgressBar output fix ([#2163](#2163)) ([85fbd58](85fbd58)) --- This PR was generated with [Release Please](https://github.com/googleapis/release-please). See [documentation](https://github.com/googleapis/release-please#release-please).
PluginManager abstraction is not needed anymore. makes less prop drilling and less code. see cloudquery/plugin-sdk#208