Skip to content

fix: Store fetch summary adjustements#426

Merged
amanenk merged 8 commits into
cloudquery:mainfrom
amanenk:store_fetch_summary_fixes
Jan 24, 2022
Merged

fix: Store fetch summary adjustements#426
amanenk merged 8 commits into
cloudquery:mainfrom
amanenk:store_fetch_summary_fixes

Conversation

@amanenk
Copy link
Copy Markdown
Contributor

@amanenk amanenk commented Jan 20, 2022 •
edited
Loading

  • Fetch summary constraint now uses alias to allow using multiple definitions of the same provider in config
  • Console fetch result reporting now use alias to report fetch results correctly
  • added createdAt fetch summary column to store time of creation of fetch summary object
  • add core_version column
  • Fetch summary start, finish, created_at columns are now nullable
    - if start is null that means that fetch hasn't started (err during initialization, etc)
    - if finish is null that means that fetch hasn't finished
  • added store of fetch summary when createProvider function failed

closes #423

@github-actions github-actions Bot added the fix label Jan 20, 2022
@amanenk amanenk requested review from bbernays, disq and roneli January 20, 2022 08:20
disq
disq requested changes Jan 20, 2022
View reviewed changes
Comment thread pkg/client/client.go
Comment thread pkg/client/client.go Outdated
@github-actions github-actions Bot added fix and removed fix labels Jan 20, 2022
@amanenk amanenk requested a review from disq January 20, 2022 13:37
disq
disq approved these changes Jan 20, 2022
View reviewed changes
Copy link
Copy Markdown
Member

@disq disq left a comment

Choose a reason for hiding this comment

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

LGTM

@amanenk
Merge remote-tracking branch 'upstream/main' into store_fetch_summary…
ffa922b 
…_fixes

# Conflicts:
#	pkg/client/client.go
#	pkg/client/fetch_test.go
@amanenk
some adjustements after merge
d85286c
@github-actions github-actions Bot added fix and removed fix labels Jan 20, 2022
roneli
roneli approved these changes Jan 23, 2022
View reviewed changes
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, another minor nit also L:489 status collides with package name fix that.

Comment thread pkg/client/client.go Outdated
@amanenk amanenk merged commit 49bd25a into cloudquery:main Jan 24, 2022
zagronitay pushed a commit to zagronitay/cloudquery that referenced this pull request Jan 24, 2022
@amanenk
fix: Store fetch summary adjustements (cloudquery#426)
d5b781e 
* added alias to fetches table,
alias used in console fetch reports,
fetch summary start and end are nullable

* test fix, fetch summary save on provider creation error.

* added created_at column to fetch summary

* test fixes

* console output adjustments, added core version column to fetches

* some adjustements after merge

* status -> fetchStatus
This was referenced May 10, 2022
Closed
Closed
erezrokah pushed a commit that referenced this pull request Aug 14, 2022
@shimonp21
test: Add dependency to terraform (#426)
80347d5
erezrokah pushed a commit that referenced this pull request Aug 14, 2022
@cq-bot
chore(main): Release v0.12.1 (#426)
cac5281
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@disq disq disq approved these changes

@bbernays bbernays Awaiting requested review from bbernays

+1 more reviewer

@roneli roneli roneli approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Fetch Summary violation on multiple providers

3 participants

@amanenk @disq @roneli