Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

.git layer and API for snapshots ( for --persist ) #92

Merged
merged 14 commits into from
Dec 8, 2021
Merged

.git layer and API for snapshots ( for --persist ) #92

merged 14 commits into from
Dec 8, 2021

Conversation

bluesign
Copy link
Collaborator

@bluesign bluesign commented Nov 3, 2021

Description

I had to do a second PR as GitHub didn't allow me to change source branch somehow.

FLIP Fest issue: CLI Feature: State management #22

Description

This PR is for issue #22

  • Adds .git compatibilty to persistence storage
  • Adds emulator HTTP API endpoint
  • Implements /emulator/snapshot/{name} command for state management
  • Implements /emulator/newBlock command to trigger new Blocks

server/backend/backend.go Outdated Show resolved Hide resolved
bluesign and others added 3 commits November 5, 2021 12:10
Co-authored-by: Gregor Gololicic <75445744+sideninja@users.noreply.github.com>
@bluesign
Copy link
Collaborator Author

bluesign commented Nov 5, 2021

@sideninja I stuck in those conflicts, this time tidy didn't help, any idea here?

@sideninja
Copy link
Contributor

I was testing the state management API and one thing that I would change is logging. Depending on the verbosity of logs you either see the first logs (non-verbose) or second (verbose). I feel that the first logs are too verbose for non-verbose mode. I also think both are missing a log that would say the state was either created or changed, so for example when I would create a new state I would get a log in sense of Created a new state snapshot with the name "test-1" and then when switching between states the log Switched to snapshot with name "test-2". This log would be the only one in a non-verbose log as the current logs are quite low-level logs that would only confuse developers. Even in verbose mode I would only keep the most critical logs, I don't think any of these logs are critical to understanding what is happening or there are some that we should keep but only in verbose mode.
Non-Verbose

INFO[0004] Got compaction priority: {level:0 score:1.73 dropPrefix:[]} 
INFO[0004] All 1 tables opened in 0s                    
INFO[0004] Replaying file id: 0 at offset: 299733       
INFO[0004] Replay took: 2.041µs 

Verbose

DEBU[0238] Storing value log head: {Fid:0 Len:29 Offset:299704} 
INFO[0238] Got compaction priority: {level:0 score:1.73 dropPrefix:[]} 
INFO[0238] Running for level: 0                         
DEBU[0238] LOG Compact. Added 242 keys. Skipped 8 keys. Iteration took: 6.844375ms 
DEBU[0238] Discard stats: map[]                         
INFO[0238] LOG Compact 0->1, del 2 tables, add 1 tables, took 45.617458ms 
INFO[0238] Compaction for level: 0 DONE                 
INFO[0238] Force compaction on level 0 done             
INFO[0238] All 1 tables opened in 0s                    
INFO[0238] Replaying file id: 0 at offset: 299733       
INFO[0238] Replay took: 1.667µs                         
DEBU[0238] Value log discard stats empty   

Another thing that I feel could be useful is for API to have some sort of response body, now you indicate success with status which is great, but it would be nice to have some response back weather a new state was created or weather the emualtor switch to that state, I could even argue it would be better to have creation of state limited to POST method and switching to PUT but I'm totally fine if it stays this way as long as there is some response so the client can now if the state was created or switched to.

As far as the code goes it's very well written, I just left some more aesthetic comments

server/emulatorApi.go Outdated Show resolved Hide resolved
storage/badger/store.go Outdated Show resolved Hide resolved
storage/badger/store.go Outdated Show resolved Hide resolved
storage/badger/store.go Outdated Show resolved Hide resolved
storage/badger/store.go Outdated Show resolved Hide resolved
storage/badger/store.go Outdated Show resolved Hide resolved
storage/badger/store.go Outdated Show resolved Hide resolved
storage/badger/store.go Outdated Show resolved Hide resolved
storage/badger/store.go Outdated Show resolved Hide resolved
storage/badger/store.go Outdated Show resolved Hide resolved
storage/badger/store.go Outdated Show resolved Hide resolved
storage/badger/store.go Outdated Show resolved Hide resolved
@sideninja
Copy link
Contributor

sideninja commented Nov 5, 2021

This is really awesome log! 🎉

Author: Flow Emulator <emulator@onflow.org>
Date:   Fri Nov 5 15:18:38 2021 +0100

    Committed Block: 06cb1d0d3446335b20e3e045ea9896a305ba90f69c42bbe456a17bb3513e6dc8
    Transaction    : 63737abf8bd5aa66af64d98f2e12405151293d4dfc2ac1f0379de27bc397fd74
    
    Arguments (2):
    
            - Argument 0: {"type":"Array","value":[]}
    
            - Argument 1: {"type":"Dictionary","value":[]}
    
    Code:
    
    transaction(publicKeys: [String], contracts: {String: String}) {
            prepare(signer: AuthAccount) {
                    let acct = AuthAccount(payer: signer)
    
                    for key in publicKeys {
                            acct.addPublicKey(key.decodeHex())
                    }
    
                    for contract in contracts.keys {
                            acct.contracts.add(name: contract, code: contracts[contract]!.decodeHex())
                    }
            }
    }
    Result:
    
            - Error Message : [1006] [Error Code: 1006] invalid proposal key: public key 0 on account f8d6e0586b0a20c7 does not have a valid signature: [Error Code: 1009] invalid envelope key: public key 0 on account f8d6e0586b0a20c7 does not have a valid signature: signature is not valid
    
            - Logs (0):
    
            - Events (0):
       

@sideninja
Copy link
Contributor

@sideninja I stuck in those conflicts, this time tidy didn't help, any idea here?

I will help you after the last commit.

@sideninja sideninja self-requested a review December 8, 2021 14:38
@sideninja sideninja merged commit 61e1a92 into onflow:master Dec 8, 2021
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.

2 participants