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.

Sign up for GitHub

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

Jump to bottom

New query pool-state command #4170

Merged
merged 4 commits into from Jul 20, 2022
Merged

New query pool-state command #4170

merged 4 commits into from Jul 20, 2022

Conversation

newhoggy
Copy link
Contributor

@newhoggy newhoggy commented Jul 12, 2022 •
edited

This obsoletes query pool-params. The command runs sub-second (rather than the 1m30s the old command takes. It also allows multiple stake pools to be queried at a time.

@newhoggy newhoggy marked this pull request as ready for review July 12, 2022 10:20
@newhoggy newhoggy force-pushed the newhoggy/new-query-pool-state-command branch 3 times, most recently from bc0e230 to b4dcd7e Compare July 12, 2022 10:25
Jimbo4350
Jimbo4350 previously requested changes Jul 12, 2022
View reviewed changes
Copy link
Contributor

@Jimbo4350 Jimbo4350 left a comment

Choose a reason for hiding this comment

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

Nice work but I think we can further simplify this and avoid adding a new command

cardano-api/src/Cardano/Api/Query.hs Outdated Show resolved Hide resolved
cardano-api/src/Cardano/Api/Shelley.hs Show resolved Hide resolved
cardano-cli/cardano-cli.cabal Outdated Show resolved Hide resolved
cardano-cli/src/Cardano/CLI/Shelley/Commands.hs Show resolved Hide resolved
@newhoggy newhoggy force-pushed the newhoggy/new-query-pool-state-command branch from b4dcd7e to 80615c8 Compare July 19, 2022 07:13
@newhoggy newhoggy force-pushed the newhoggy/new-query-pool-state-command branch 3 times, most recently from 4507f38 to 8c1d6c9 Compare July 19, 2022 07:24
@newhoggy newhoggy requested a review from Jimbo4350 July 19, 2022 07:36
@CarlosLopezDeLara CarlosLopezDeLara mentioned this pull request Jul 19, 2022
Closed
4 tasks
@newhoggy newhoggy force-pushed the newhoggy/new-query-pool-state-command branch from 0df5d04 to e60d113 Compare July 19, 2022 12:04
Jimbo4350
Jimbo4350 previously requested changes Jul 19, 2022
View reviewed changes
Copy link
Contributor

@Jimbo4350 Jimbo4350 left a comment

Choose a reason for hiding this comment

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

Two minor fixes

cardano-api/src/Cardano/Api/Query.hs Outdated Show resolved Hide resolved
cardano-cli/src/Cardano/CLI/Shelley/Run/Query.hs Outdated Show resolved Hide resolved
Jimbo4350
Jimbo4350 reviewed Jul 19, 2022
View reviewed changes
releases.json Outdated Show resolved Hide resolved
@newhoggy newhoggy force-pushed the newhoggy/new-query-pool-state-command branch 3 times, most recently from 575d6cc to c6202f1 Compare July 19, 2022 22:53
@newhoggy newhoggy requested a review from Jimbo4350 July 19, 2022 22:53
@newhoggy newhoggy dismissed Jimbo4350’s stale review July 19, 2022 22:54

comments addressed

Jimbo4350
Jimbo4350 approved these changes Jul 19, 2022
View reviewed changes
Copy link
Contributor

@Jimbo4350 Jimbo4350 left a comment

Choose a reason for hiding this comment

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

Good work! LGTM

@newhoggy newhoggy force-pushed the newhoggy/new-query-pool-state-command branch from c6202f1 to f0475c9 Compare July 19, 2022 23:26
@newhoggy
Copy link
Contributor Author

bors r+

iohk-bors bot added a commit that referenced this pull request Jul 19, 2022
@iohk-bors @newhoggy
Merge #4170
32f5bfe 
4170: New query pool-state command r=newhoggy a=newhoggy

This obsoletes `query pool-params`.  The command runs sub-second (rather than the `1m30s` the old command takes.  It also allows multiple stake pools to be queried at a time.

* Requires IntersectMBO/ouroboros-network#3890
* Requires IntersectMBO/cardano-ledger#2908

Co-authored-by: John Ky <john.ky@iohk.io>
@iohk-bors
Copy link
Contributor

iohk-bors bot commented Jul 20, 2022

Build failed:

@newhoggy
Copy link
Contributor Author

bors r+

@iohk-bors
Copy link
Contributor

iohk-bors bot commented Jul 20, 2022

Build succeeded:

@iohk-bors iohk-bors bot merged commit 2be07bb into master Jul 20, 2022
@iohk-bors iohk-bors bot deleted the newhoggy/new-query-pool-state-command branch July 20, 2022 00:56
@CarlosLopezDeLara CarlosLopezDeLara mentioned this pull request Jul 21, 2022
Open
40 tasks
@newhoggy newhoggy mentioned this pull request Jul 25, 2022
Closed
@CarlosLopezDeLara CarlosLopezDeLara mentioned this pull request Aug 12, 2022
Closed
@saratomaz saratomaz mentioned this pull request Oct 28, 2022
Open
@CarlosLopezDeLara CarlosLopezDeLara mentioned this pull request Nov 3, 2022
Open
19 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@Jimbo4350 Jimbo4350 Jimbo4350 approved these changes

@dcoutts dcoutts Awaiting requested review from dcoutts

@erikd erikd Awaiting requested review from erikd

@JaredCorduan JaredCorduan Awaiting requested review from JaredCorduan

@deepfire deepfire Awaiting requested review from deepfire

@denisshevchenko denisshevchenko Awaiting requested review from denisshevchenko

@jutaro jutaro Awaiting requested review from jutaro

@coot coot Awaiting requested review from coot

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
2 participants
@newhoggy @Jimbo4350