Skip to content

feat(cli): Print and log sources and destinations versions#6842

Merged
kodiakhq[bot] merged 5 commits intocloudquery:mainfrom
erezrokah:feat/print_source_dests_with_versions
Jan 17, 2023
Merged

feat(cli): Print and log sources and destinations versions#6842
kodiakhq[bot] merged 5 commits intocloudquery:mainfrom
erezrokah:feat/print_source_dests_with_versions

Conversation

@erezrokah
Copy link
Copy Markdown
Member

Summary

To go with cloudquery/plugin-sdk#609.
Examples
image
image

func syncConnectionV2(ctx context.Context, cqDir string, sourceClient *clients.SourceClient, sourceSpec specs.Source, destinationsSpecs []specs.Destination, uid string, noMigrate bool) error {
var err error
destinationNames := make([]string, len(destinationsSpecs))
destinationStrings := make([]string, len(destinationsSpecs))
Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

zerolog supports Stringers and Stringer, however Stringers is not very useful as we need to create that array manually (we can't cast to an array directly)

@hermanschaaf
Copy link
Copy Markdown
Contributor

@erezrokah There were a couple of CI errors reported here, FYI

@erezrokah
Copy link
Copy Markdown
Member Author

@erezrokah There were a couple of CI errors reported here, FYI

👍 Those should be fixed after cloudquery/plugin-sdk#609 as we need the String() method on the specs

kodiakhq bot pushed a commit to cloudquery/plugin-sdk that referenced this pull request Jan 17, 2023


A user recently ran into some issues since they thought they used a newer version of a plugin, but actually used an old one.
This PR adds `String()` methods to specs so we can use them in logs/errors.

CLI PR cloudquery/cloudquery#6842

---
@erezrokah erezrokah changed the title feat(cli): Print/log sources, destinations versions feat(cli): Print and log sources and destinations versions Jan 17, 2023
@erezrokah erezrokah force-pushed the feat/print_source_dests_with_versions branch from 6904918 to 0b1d866 Compare January 17, 2023 11:00
cli/cmd/sync2.go Outdated
if !noMigrate {
fmt.Println("Starting migration for:", sourceSpec.Name, "->", sourceSpec.Destinations)
log.Info().Str("source", sourceSpec.Name).Strs("destinations", sourceSpec.Destinations).Msg("Start migration")
fmt.Printf("Starting migration for:%s->%s\n", sourceSpec.VersionString(), destinationStrings)
Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Printf is less error prone as our linter errors if you use %s and don't pass a string

@erezrokah
Copy link
Copy Markdown
Member Author

@hermanschaaf this should be ready for another review after using the new SDK

Copy link
Copy Markdown
Contributor

@hermanschaaf hermanschaaf left a comment

Choose a reason for hiding this comment

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

Looks good, let's just fix the spacing in that one message before merging :)

erezrokah and others added 2 commits January 17, 2023 13:48
Co-authored-by: Herman Schaaf <hermanschaaf@gmail.com>
@erezrokah erezrokah added automerge Automatically merge once required checks pass priority merge labels Jan 17, 2023
@kodiakhq kodiakhq bot merged commit b32363b into cloudquery:main Jan 17, 2023
kodiakhq bot pushed a commit that referenced this pull request Jan 17, 2023
🤖 I have created a release *beep* *boop*
---


## [2.2.0](cli-v2.1.1...cli-v2.2.0) (2023-01-17)


### Features

* **cli:** Print and log sources and destinations versions ([#6842](#6842)) ([b32363b](b32363b))

---
This PR was generated with [Release Please](https://github.com/googleapis/release-please). See [documentation](https://github.com/googleapis/release-please#release-please).
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

automerge Automatically merge once required checks pass

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants