Connect Pool DB with Api #881
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Anviking
force-pushed
the
anviking/713/connect-db-with-api
branch
4 times, most recently
from
1337b43
to
6c15d00
Compare
KtorZ
reviewed
Oct 22, 2019
KtorZ
reviewed
Oct 22, 2019
Anviking
commented
Oct 22, 2019
KtorZ
reviewed
Oct 22, 2019
KtorZ
reviewed
Oct 22, 2019
KtorZ
reviewed
Oct 22, 2019
KtorZ
reviewed
Oct 22, 2019
KtorZ
reviewed
Oct 22, 2019
Anviking
force-pushed
the
anviking/713/connect-db-with-api
branch
from
6c15d00
to
fd70b03
Compare
Anviking
force-pushed
the
anviking/713/connect-db-with-api
branch
from
6a961e5
to
2b98247
Compare
KtorZ
reviewed
Oct 22, 2019
KtorZ
reviewed
Oct 22, 2019
|
|
||
| -- StakePool | ||
| PoolSqlite.withDBLayer cfg tr (poolDBPath databaseDir) $ | ||
| \splDB -> do |
There was a problem hiding this comment.
Because we have a rule saying lines must be 80 characters or less 😬 This one would have been 81.
Maybe importing Cardano.Pool.DB.Sqlite as Pool instead of PoolSqlite would actually be nice though…
There was a problem hiding this comment.
Hmm. Yes. But the guidelines are... Guidelines. When it makes sense, we can break them and we sometimes do.
Anviking
force-pushed
the
anviking/713/connect-db-with-api
branch
6 times, most recently
from
5294f42
to
91fcdaa
Compare
KtorZ
reviewed
Oct 23, 2019
| _ -> return Nothing | ||
|
|
||
| progress :: BlockHeader -> BlockHeader -> Percentage | ||
| progress tip target = |
There was a problem hiding this comment.
Why not use the progress calculation from the primitive types?
There was a problem hiding this comment.
- This
blockHeight-calculation would return 100% when we are at the node tip. I.e. when thestake-pool listresults become available.syncProgresscompare to the current time. If the node tip is behind the current time, the progress might then jump from 50% to, show results. - This
blockHeightapproach is simple and gives us (seemingly) perfectly linear increase in progress. syncProgressreturnsSyncProgress = Ready | Restoring, which we don't want.syncProgresshas atolerancewhich we don't want. Here we know that we are not in sync, and want to show the exact progress to the user. Even if that would happen to be 99% or 100%.
| PoolSqlite.withDBLayer cfg tr (poolDBPath databaseDir) $ | ||
| \splDB -> do | ||
|
|
||
| -- TODO: I believe @KTorZ wants us to use @WorkerRegistry@ |
There was a problem hiding this comment.
Not WorkerRegistry. There's no need for this. But newWorker would be good. With void $ forkIO, we are just firing a thread in the wild and we have no control over it..
There was a problem hiding this comment.
Separate PR? And can we talk about it first or maybe you want to do do that?
KtorZ
reviewed
Oct 23, 2019
Anviking
force-pushed
the
anviking/713/connect-db-with-api
branch
3 times, most recently
from
1d524da
to
44741c3
Compare
Anviking
force-pushed
the
anviking/713/connect-db-with-api
branch
from
f3136c4
to
a5e657f
Compare
|
bors r+ |
iohk-bors bot
added a commit
that referenced
this pull request
Oct 23, 2019
881: Connect Pool DB with Api r=Anviking a=Anviking # Issue Number #713 # Overview - [x] I have implemented a `GET /stake-pool` that reads and combines metrics from `Pool.DB` - [x] TODO: Figure out when we are unsynced - [ ] TODO: Look for things to test if possible in this PR # Comments ### Self-node with a very short epoch-length (results are unavailable across epoch boundaries)  ### Testnet syncing which is interrupted and resumed  <!-- Additional comments or screenshots to attach if any --> <!-- Don't forget to: ✓ Self-review your changes to make sure nothing unexpected slipped through ✓ Assign yourself to the PR ✓ Assign one or several reviewer(s) ✓ Once created, link this PR to its corresponding ticket ✓ Acknowledge any changes required to the Wiki --> Co-authored-by: Johannes Lund <johannes.lund@iohk.io> Co-authored-by: KtorZ <matthias.benkort@gmail.com>
Build failed |
KtorZ
force-pushed
the
anviking/713/connect-db-with-api
branch
from
4540a00
to
247270c
Compare
|
bors r+ |
iohk-bors bot
added a commit
that referenced
this pull request
Oct 24, 2019
881: Connect Pool DB with Api r=KtorZ a=Anviking # Issue Number #713 # Overview - [x] I have implemented a `GET /stake-pool` that reads and combines metrics from `Pool.DB` - [x] TODO: Figure out when we are unsynced - [ ] TODO: Look for things to test if possible in this PR # Comments ### Self-node with a very short epoch-length (results are unavailable across epoch boundaries)  ### Testnet syncing which is interrupted and resumed  <!-- Additional comments or screenshots to attach if any --> <!-- Don't forget to: ✓ Self-review your changes to make sure nothing unexpected slipped through ✓ Assign yourself to the PR ✓ Assign one or several reviewer(s) ✓ Once created, link this PR to its corresponding ticket ✓ Acknowledge any changes required to the Wiki --> Co-authored-by: Johannes Lund <johannes.lund@iohk.io> Co-authored-by: KtorZ <matthias.benkort@gmail.com>
Build failed |
fixup: use minBound :: Percentage fixup: convert -> count fixup whitespace spl Add more detailed TODOs for detecting if we are unsynced Fixup: accidental diff Other approach
fixup: f -> mkApiStakePool fixup: error in bridge listStakePools fixup: use withDBLayer Other approach
…assertions NOTE: 4xx code are for CLIENTS errors. Not being able to answer because we aren't synced is not a client error.
…ackoff in 'nextBlocks' too
KtorZ
force-pushed
the
anviking/713/connect-db-with-api
branch
from
247270c
to
3adffb1
Compare
|
bors r+ |
iohk-bors bot
added a commit
that referenced
this pull request
Oct 24, 2019
881: Connect Pool DB with Api r=KtorZ a=Anviking # Issue Number #713 # Overview - [x] I have implemented a `GET /stake-pool` that reads and combines metrics from `Pool.DB` - [x] TODO: Figure out when we are unsynced - [ ] TODO: Look for things to test if possible in this PR # Comments ### Self-node with a very short epoch-length (results are unavailable across epoch boundaries)  ### Testnet syncing which is interrupted and resumed  <!-- Additional comments or screenshots to attach if any --> <!-- Don't forget to: ✓ Self-review your changes to make sure nothing unexpected slipped through ✓ Assign yourself to the PR ✓ Assign one or several reviewer(s) ✓ Once created, link this PR to its corresponding ticket ✓ Acknowledge any changes required to the Wiki --> Co-authored-by: Johannes Lund <johannes.lund@iohk.io> Co-authored-by: KtorZ <matthias.benkort@gmail.com>
Build succeeded |
Anviking
commented
Oct 24, 2019
| \situation. Here's some information about what happened: \n" | ||
| o `shouldBe` mempty | ||
| c `shouldBe` ExitFailure 1 | ||
| eventually $ do |
There was a problem hiding this comment.
😱 I was wondering if there was something as simple as eventually
3 tasks
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Issue Number
#713
Overview
GET /stake-poolthat reads and combines metrics fromPool.DBComments
Self-node with a very short epoch-length (results are unavailable across epoch boundaries)
Testnet syncing which is interrupted and resumed