Skip to content

fix: snapshotter's failure is not propogated#16588

Merged
facundomedica merged 5 commits intocosmos:mainfrom
yihuang:snapshot-error
Jun 17, 2023
Merged

fix: snapshotter's failure is not propogated#16588
facundomedica merged 5 commits intocosmos:mainfrom
yihuang:snapshot-error

Conversation

@yihuang
Copy link
Copy Markdown
Collaborator

@yihuang yihuang commented Jun 16, 2023 •
edited
Loading

Description

A invalid export call will create an empty snapshot, instead of a failure.

Solution:

  • create a dummy pipe to propogate the error
  • add unit tests to verify it

Author Checklist

All items are required. Please add a note to the item if the item is not applicable and
please add links to any relevant follow up issues.

I have...

  • included the correct type prefix in the PR title
  • added ! to the type prefix if API or client breaking change
  • targeted the correct branch (see PR Targeting)
  • provided a link to the relevant issue or specification
  • followed the guidelines for building modules
  • included the necessary unit and integration tests
  • added a changelog entry to CHANGELOG.md
  • included comments for documenting Go code
  • updated the relevant documentation or specification
  • reviewed "Files changed" and left comments if necessary
  • confirmed all CI checks have passed

Reviewers Checklist

All items are required. Please add a note if the item is not applicable and please add
your handle next to the items reviewed if you only reviewed selected items.

I have...

  • confirmed the correct type prefix in the PR title
  • confirmed ! in the type prefix if API or client breaking change
  • confirmed all author checklist items have been addressed
  • reviewed state machine logic
  • reviewed API design and naming
  • reviewed documentation is accurate
  • reviewed tests and test coverage
  • manually tested (if applicable)

fix: snapshotter failure is not propogated
8893c8c 
Solution:
- create a dummy pipe to propogate the error
- add unit tests to verify it
@yihuang yihuang requested a review from a team June 16, 2023 08:06
@ghost ghost requested review from a team, alexanderbez and julienrbrt and removed request for a team June 16, 2023 08:07
yihuang
yihuang commented Jun 16, 2023
View reviewed changes
@facundomedica facundomedica enabled auto-merge June 16, 2023 15:30
julienrbrt
julienrbrt approved these changes Jun 17, 2023
View reviewed changes
Copy link
Copy Markdown
Contributor

@julienrbrt julienrbrt left a comment

Choose a reason for hiding this comment

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

utACK. Could you fix linting?

@yihuang
Copy link
Copy Markdown
Collaborator Author

yihuang commented Jun 17, 2023

utACK. Could you fix linting?

done

@facundomedica facundomedica added this pull request to the merge queue Jun 17, 2023
Merged via the queue into cosmos:main with commit aeccbc9 Jun 17, 2023
mergify bot pushed a commit that referenced this pull request Jun 17, 2023
@mergify
fix: snapshotter's failure is not propogated (#16588)
b3addca 
(cherry picked from commit aeccbc9)
@mergify mergify bot mentioned this pull request Jun 17, 2023
Closed
mergify bot pushed a commit that referenced this pull request Jun 17, 2023
@mergify
fix: snapshotter's failure is not propogated (#16588)
9775afc 
(cherry picked from commit aeccbc9)

# Conflicts:
#	snapshots/manager_test.go
#	store/CHANGELOG.md
mergify bot pushed a commit that referenced this pull request Jun 17, 2023
@mergify
fix: snapshotter's failure is not propogated (#16588)
074b4e3 
(cherry picked from commit aeccbc9)

# Conflicts:
#	store/CHANGELOG.md
@mergify mergify bot mentioned this pull request Jun 17, 2023
Merged
@mergify mergify bot mentioned this pull request Jun 17, 2023
Merged
@yihuang yihuang deleted the snapshot-error branch June 17, 2023 16:22
tac0turtle pushed a commit that referenced this pull request Jun 19, 2023
@mergify
fix: snapshotter's failure is not propogated (backport #16588) (#16603)
40c4daf 
Co-authored-by: yihuang <huang@crypto.com>
tac0turtle pushed a commit that referenced this pull request Jun 19, 2023
@mergify
fix: snapshotter's failure is not propogated (backport #16588) (#16604)
56df2b2 
Co-authored-by: yihuang <huang@crypto.com>
GAtom22 pushed a commit to evmos/cosmos-sdk that referenced this pull request Jul 12, 2023
@mergify @GAtom22
fix: snapshotter's failure is not propogated (backport cosmos#16588) (c…
6f4ead9 
…osmos#16604)

Co-authored-by: yihuang <huang@crypto.com>
GAtom22 pushed a commit to evmos/cosmos-sdk that referenced this pull request Jul 12, 2023
@mergify @GAtom22
fix: snapshotter's failure is not propogated (backport cosmos#16588) (c…
dcb188f 
…osmos#16604)

Co-authored-by: yihuang <huang@crypto.com>
cmwaters added a commit to celestiaorg/cosmos-sdk that referenced this pull request Aug 15, 2023
@cmwaters @julienrbrt
@mergify @odeke-em @mmsqe @tac0turtle @KyleMoser @rootulp
chore: bump sdk to v0.46.14 and tm to v1.26.2-tm-v0.34.28 (#340)
64adc6e 
* build(deps): bump cometbft to v0.34.29 (cosmos#16553)

* fix(x/auth): ensure nil .BaseAccounts are reported in ModuleAccount.Validate (backport cosmos#16554) (cosmos#16570)

Co-authored-by: Emmanuel T Odeke <emmanuel@orijtech.com>
Co-authored-by: Julien Robert <julien@rbrt.fr>

* fix: snapshotter's failure is not propogated (backport cosmos#16588) (cosmos#16604)

Co-authored-by: yihuang <huang@crypto.com>

* feat: add custom max gas for block for sim config (backport cosmos#16656) (cosmos#16731)

Co-authored-by: mmsqe <mavis@crypto.com>
Co-authored-by: marbar3778 <marbar3778@yahoo.com>

* fix: accaddr cachefix (backport cosmos#15433) (cosmos#16823)

Co-authored-by: KyleMoser <KyleMoser@users.noreply.github.com>
Co-authored-by: HuangYi <huang@crypto.com>

* fix(cli): improve `prune` command ux (backport cosmos#16856) (cosmos#16876)

Co-authored-by: Julien Robert <julien@rbrt.fr>

* fix: query tx events with `>=` and `<=` operators (cosmos#16994)

* docs: change bank multi-send command description (backport cosmos#16950) (cosmos#17019)

* chore: prepare v0.46.14 (cosmos#16879)

* refactor: add MigrateHandler to allow reuse migrate genesis related function  (backport cosmos#17296) (cosmos#17301)

Co-authored-by: mmsqe <mavis@crypto.com>
Co-authored-by: Julien Robert <julien@rbrt.fr>

* fix: resolve migration map in MigrateHandler (cosmos#17301) (cosmos#17302)

* feat(x/gov): Emit VoterAddr (backport cosmos#17354) (cosmos#17357)

Co-authored-by: Devon Bear <itsdevbear@berachain.com>
Co-authored-by: Julien Robert <julien@rbrt.fr>

* feat(x/gov): add MsgSubmitProposal SetMsgs method (backport cosmos#17387) (cosmos#17388)

Co-authored-by: Julien Robert <julien@rbrt.fr>

---------

Co-authored-by: Julien Robert <julien@rbrt.fr>
Co-authored-by: mergify[bot] <37929162+mergify[bot]@users.noreply.github.com>
Co-authored-by: Emmanuel T Odeke <emmanuel@orijtech.com>
Co-authored-by: yihuang <huang@crypto.com>
Co-authored-by: mmsqe <mavis@crypto.com>
Co-authored-by: marbar3778 <marbar3778@yahoo.com>
Co-authored-by: KyleMoser <KyleMoser@users.noreply.github.com>
Co-authored-by: Rootul P <rootulp@gmail.com>
Co-authored-by: Devon Bear <itsdevbear@berachain.com>
@faddat faddat mentioned this pull request Sep 5, 2023
Closed
19 tasks
@faddat faddat mentioned this pull request Oct 15, 2023
Closed
19 tasks
@coderabbitai coderabbitai bot mentioned this pull request Dec 8, 2023
Merged
@0xstepit 0xstepit mentioned this pull request Jul 1, 2024
Merged
JeancarloBarrios pushed a commit to agoric-labs/cosmos-sdk that referenced this pull request Sep 28, 2024
@mergify @JeancarloBarrios
fix: snapshotter's failure is not propogated (backport cosmos#16588) (c…
e7b445a 
…osmos#16604)

Co-authored-by: yihuang <huang@crypto.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@alexanderbez alexanderbez Awaiting requested review from alexanderbez

3 more reviewers

@facundomedica facundomedica facundomedica approved these changes

@tac0turtle tac0turtle tac0turtle approved these changes

@julienrbrt julienrbrt julienrbrt approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

@facundomedica facundomedica

@tac0turtle tac0turtle

Labels

C:Store

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

4 participants

@yihuang @facundomedica @tac0turtle @julienrbrt