Skip to content

Shapshots list admin endpoint #241

Merged
devbugging merged 5 commits into
onflow:masterfrom
bluesign:snapshotList
Jan 16, 2023
Merged

Shapshots list admin endpoint #241
devbugging merged 5 commits into
onflow:masterfrom
bluesign:snapshotList

Conversation

@bluesign
Copy link
Copy Markdown
Collaborator

Closes #228

Description

I was thinking 2 endpoints:

GET /snapshots to list all snapshots, in any case we should add, I guess, it seems pretty nice to have.
GET /snapshots/{snapshotID} Retrieve existing one is also not a problem. ( also 404 on fail )

But it will not be backwards compatible. But if we decide like that ( and move create to POST verb ) I can update the PR


For contributor use:

  • Targeted PR against master branch
  • Linked to GitHub issue with discussion and accepted design OR link to spec that describes this work
  • Code follows the standards mentioned here
  • Updated relevant documentation
  • Re-reviewed Files changed in the GitHub PR explorer
  • Added appropriate labels

@codecov-commenter
Copy link
Copy Markdown

Codecov Report

Merging #241 (59f41eb) into master (dddefb8) will increase coverage by 1.17%.
The diff coverage is 7.69%.

@@            Coverage Diff             @@
##           master     #241      +/-   ##
==========================================
+ Coverage   50.96%   52.14%   +1.17%     
==========================================
  Files          29       26       -3     
  Lines        2372     2054     -318     
==========================================
- Hits         1209     1071     -138     
+ Misses       1019      857     -162     
+ Partials      144      126      -18     
Flag Coverage Δ
unittests 52.14% <7.69%> (+1.17%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

Impacted Files Coverage Δ
server/emulator.go 12.04% <7.69%> (-0.81%) ⬇️
server/storage.go 41.02% <0.00%> (-8.98%) ⬇️
server/server.go 48.50% <0.00%> (-0.74%) ⬇️
blocks.go 60.00% <0.00%> (ø)
headers.go 21.42% <0.00%> (ø)
storage/badger/store.go
storage/badger/encoding.go
storage/badger/changelog.go
storage/badger/keys.go
storage/badger/config.go
... and 3 more

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

@devbugging
Copy link
Copy Markdown
Collaborator

I would say that if we are doing changes in API I would make the API to be more RESTful:
GET /snapshots list all
POST /snapshots create a new snapshot, and provide a name in body
PUT /snapshots/{id} restore a specific snapshot

@devbugging
Copy link
Copy Markdown
Collaborator

@bluesign is this complete and ready for review? sorry I guess we missed it.

@bluesign
Copy link
Copy Markdown
Collaborator Author

Yeah should be ready, Sorry I forgot this too

Comment thread server/emulator.go
Comment thread server/emulator.go
Comment thread server/emulator.go Outdated
Comment thread server/emulator.go Outdated
@devbugging
Copy link
Copy Markdown
Collaborator

@bluesign left a couple of questions, the biggest one is a check whether the store is of type badger, and also generally to think how will this work with the store refactor that happened meanwhile.

@bluesign
Copy link
Copy Markdown
Collaborator Author

@sideninja this one also I will handle Monday if not urgent

@devbugging
Copy link
Copy Markdown
Collaborator

@bluesign are we ok to merge?

@bluesign
Copy link
Copy Markdown
Collaborator Author

@bluesign are we ok to merge?

sorry I missed this, this can be merged.

@devbugging devbugging merged commit 2559209 into onflow:master Jan 16, 2023
@devbugging
Copy link
Copy Markdown
Collaborator

Could we add this to the CLI as a command 🙏

@bluesign
Copy link
Copy Markdown
Collaborator Author

@sideninja very good idea, I will make a PR

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.

Split snapshot management into multiple HTTP endpoints

3 participants