Skip to content

node to client api changes #1962

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
Merged
merged 2 commits into from
Apr 17, 2020
Merged

node to client api changes #1962

merged 2 commits into from
Apr 17, 2020

Conversation

coot
Copy link
Contributor

@coot coot commented Apr 16, 2020

Small changes to NodeToClient api and LocalStateQuery protocol type signatures.

Related to #1950

  • Added null peers in NodeToNode module
  • Add kinds to type signatures of LocalQueryProtocol

@coot coot requested review from MarcFontaine and dcoutts April 16, 2020 10:26
@coot coot changed the title coot/node to client api node to client api changes Apr 16, 2020
MarcFontaine
MarcFontaine approved these changes Apr 16, 2020
View reviewed changes
Copy link
Contributor

@MarcFontaine MarcFontaine left a comment

Choose a reason for hiding this comment

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

Maybe forever $ threadDelay 43200 should be replaced with something better.

@coot
Copy link
Contributor Author

coot commented Apr 16, 2020 •
edited
Loading

@MarcFontaine any ideas? Note that we need to trick GHC no to throw BlockIndefinitely... errors or FixIOException.

@coot
Copy link
Contributor Author

coot commented Apr 16, 2020

bors merge

@MarcFontaine
Copy link
Contributor

Right. I did not think about BlockIndefinitely.
One could use a global MVar to block everything.
In any case if we add a abstraction for waitForever we can change the implementation in a single point.

@coot
Copy link
Contributor Author

coot commented Apr 16, 2020

One could use a global MVar to block everything.

But that's ugly :D; we even avoided such global state in Win32-network in favor of withIOManager :: (IOManager -> IO a) -> IO a.

In any case if we add a abstraction for waitForever we can change the implementation in a single point.

untilTheCowsComeHome serves this purpose :), we only have one more such call in ntp-client and that's about it. AFAIK, other repos are also only using this trick because they don't use the null peers yet.

iohk-bors bot added a commit that referenced this pull request Apr 16, 2020
@iohk-bors @mrBliss @coot
Merge #1928 #1962
635f38e 
1928: Fix thunks in Shelley r=mrBliss a=mrBliss

Fixes #1558.

1962: node to client api changes r=coot a=coot

Small changes to NodeToClient api and LocalStateQuery protocol type signatures.

Related to #1950

- Added null peers in NodeToNode module
- Add kinds to type signatures of LocalQueryProtocol


Co-authored-by: Thomas Winant <thomas@well-typed.com>
Co-authored-by: Marcin Szamotulski <profunctor@pm.me>
@mrBliss
Copy link
Contributor

mrBliss commented Apr 16, 2020

bors cancel

@mrBliss mrBliss mentioned this pull request Apr 16, 2020
Merged
@iohk-bors
Copy link
Contributor

iohk-bors bot commented Apr 16, 2020

This PR was included in a batch that was canceled, it will be automatically retried

@mrBliss
Copy link
Contributor

mrBliss commented Apr 16, 2020

bors r-

@iohk-bors
Copy link
Contributor

iohk-bors bot commented Apr 16, 2020

Canceled

@mrBliss
Copy link
Contributor

mrBliss commented Apr 16, 2020

This PR was included in a batch that was canceled, it will be automatically retried

Seriously? This PR was closed and the job cancelled, and still it retries? Useless 😩

@coot
Copy link
Contributor Author

coot commented Apr 16, 2020

@MarcFontaine why did you closed it?

@coot coot reopened this Apr 16, 2020
@coot
Copy link
Contributor Author

coot commented Apr 16, 2020

bors merge

@MarcFontaine
Copy link
Contributor

Sorry. I just click on the wrong button I guess.
I did not notice that I closed it

iohk-bors bot added a commit that referenced this pull request Apr 16, 2020
@iohk-bors @coot
Merge #1962
6c16192 
1962: node to client api changes r=coot a=coot

Small changes to NodeToClient api and LocalStateQuery protocol type signatures.

Related to #1950

- Added null peers in NodeToNode module
- Add kinds to type signatures of LocalQueryProtocol


Co-authored-by: Marcin Szamotulski <profunctor@pm.me>
@mrBliss
Copy link
Contributor

mrBliss commented Apr 16, 2020

I have found out why the tests are taking so long: 0ae688d. I'm going to cancel this bors job because it will time out anyway so that #1928, which will fix the problem, can be merged. After that, we should be able to merge this PR without problems. Apologies in advance.

@mrBliss
Copy link
Contributor

mrBliss commented Apr 16, 2020

bors cancel

@mrBliss
Copy link
Contributor

mrBliss commented Apr 16, 2020

bors r-

@iohk-bors
Copy link
Contributor

iohk-bors bot commented Apr 16, 2020

Canceled

@coot
Copy link
Contributor Author

coot commented Apr 16, 2020

thanks @mrBliss for looking into this.

coot added 2 commits April 16, 2020 17:49
@coot
Add kinds to type signatures of LocalQueryProtocol
c23ef52 
Makeing query kind explicit make it much easier to spot what is the
goal; Otherwise it is burried deep in the `Protocol` instance.
@coot
Added null peers in NodeToNode module
4e6791b 
`Peer` can be pulgged directly in `MuxPeer`; we could add a null
`MuxPeerRaw`, but this will be deprecated sooner (or later).
@coot coot force-pushed the coot/node-to-client-api branch from 32f0c9e to 4e6791b Compare April 16, 2020 15:49
@mrBliss
Copy link
Contributor

mrBliss commented Apr 16, 2020

@coot Make sure to rebase on master before trying to bors it.

@coot
Copy link
Contributor Author

coot commented Apr 16, 2020

I did :):

git log
...
807f86ace Merge #1928                               (origin/master, origin/bors/staging, origin/HEAD)

@coot
Copy link
Contributor Author

coot commented Apr 17, 2020

borse merge

@coot
Copy link
Contributor Author

coot commented Apr 17, 2020

bors merge

@iohk-bors
Copy link
Contributor

iohk-bors bot commented Apr 17, 2020

Build succeeded

@iohk-bors iohk-bors bot merged commit 28f821a into master Apr 17, 2020
@iohk-bors iohk-bors bot deleted the coot/node-to-client-api branch April 17, 2020 08:21
@coot coot added the node-to-client Issues & PRs related to node-to-client protocols label Apr 18, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@dcoutts dcoutts Awaiting requested review from dcoutts

1 more reviewer

@MarcFontaine MarcFontaine MarcFontaine approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

node-to-client Issues & PRs related to node-to-client protocols

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@coot @MarcFontaine @mrBliss