Skip to content

fix: Add safeguard for drop provider#580

Merged
disq merged 2 commits into
cloudquery:mainfrom
disq:fix/drop-provider-safeguard
Apr 13, 2022
Merged

fix: Add safeguard for drop provider#580
disq merged 2 commits into
cloudquery:mainfrom
disq:fix/drop-provider-safeguard

Conversation

@disq
Copy link
Copy Markdown
Member

@disq disq commented Apr 13, 2022

No description provided.

@disq disq requested review from a team, bbernays and roneli April 13, 2022 09:09
@github-actions github-actions Bot added the fix label Apr 13, 2022
Copy link
Copy Markdown
Contributor

@roneli roneli left a comment

Choose a reason for hiding this comment

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

LGTM had 1 nit.

Comment thread cmd/provider.go Outdated
"last-update is the duration from current time we want to remove resources from the database. "+
"For example 24h will remove all resources that were not update in last 24 hours. Duration is a string with optional unit suffix such as \"2h45m\" or \"7d\"")
providerRemoveStaleCmd.Flags().BoolVar(&dryRun, "dry-run", true, "")
providerDropCmd.Flags().BoolVar(&iAmSure, "i-am-sure", false, "Really drop tables for the provider")
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.

nit: --force ? --approve-drop ?

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

I like --force

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.

Done!

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.

...but --force implies "it's normally possible to do it without forcing, but for some reason, in this instance, we can't" which is not the case here.

@disq disq merged commit 4c2b3e7 into cloudquery:main Apr 13, 2022
@disq disq deleted the fix/drop-provider-safeguard branch April 13, 2022 13:01
TinLe pushed a commit to TinLe/cloudquery that referenced this pull request Apr 13, 2022
* upstream/main:
  fix: Add safeguard for drop provider (cloudquery#580)
  feat: Remote config (cloudquery#568)
  fix: Handle panic level diags (cloudquery#579)
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