Skip to content
This repository has been archived by the owner on Nov 6, 2020. It is now read-only.

Determine real-time HTTP connected status #3335

Merged
merged 4 commits into from
Nov 11, 2016
Merged

Conversation

jacogr
Copy link
Contributor

@jacogr jacogr commented Nov 10, 2016

Closes https://github.com/ethcore/parity/issues/3330

(Will swap to parity_ping once available)

@jacogr jacogr added A0-pleasereview 🤓 Pull request needs code review. M6-rpcapi 📣 RPC API. labels Nov 10, 2016
@derhuerst
Copy link
Contributor

Looks good imo. Isn't polling once a second pretty often? We already have another polling mechanism in place (HTTP HEAD /), which we might combine with this one.

@derhuerst derhuerst added A8-looksgood 🦄 Pull request is reviewed well. and removed A0-pleasereview 🤓 Pull request needs code review. labels Nov 10, 2016
@jacogr
Copy link
Contributor Author

jacogr commented Nov 10, 2016

This is for the HTTP API, nothing to do with the UI at all. isConnected gets handled properly for the WS API (since we get feedback and make a connection, so actually there is a discrepancy between the two transports).

The UI doesn't use HTTP anywhere, while Dapps don't use the ping nor currently have feedback on the actual node. (As reported in the linked issue)

API call to be replaced with parity_ping (once available - and probably backported to beta)

@gavofyork gavofyork added A7-looksgoodtestsfail 🤖 Pull request is reviewed well, but cannot be merged due to tests failing. and removed A8-looksgood 🦄 Pull request is reviewed well. labels Nov 10, 2016
@jacogr
Copy link
Contributor Author

jacogr commented Nov 10, 2016

Merged in master for test failures.

@jacogr jacogr added A0-pleasereview 🤓 Pull request needs code review. and removed A7-looksgoodtestsfail 🤖 Pull request is reviewed well, but cannot be merged due to tests failing. labels Nov 10, 2016
@gavofyork
Copy link
Contributor

tests still failing.

@jacogr jacogr added A3-inprogress ⏳ Pull request is in progress. No review needed at this stage. and removed A0-pleasereview 🤓 Pull request needs code review. labels Nov 10, 2016
@jacogr
Copy link
Contributor Author

jacogr commented Nov 10, 2016

Ok, looking at it yet again, these are real failures - missed a beat. Moving to in-progress until properly fixed.

@jacogr jacogr added A0-pleasereview 🤓 Pull request needs code review. and removed A3-inprogress ⏳ Pull request is in progress. No review needed at this stage. labels Nov 10, 2016
@jacogr
Copy link
Contributor Author

jacogr commented Nov 10, 2016

Failing tests finally fixed. (Expecting a sequence of mocked calls with polling timeouts in-between is not the best match for consistency)

@gavofyork gavofyork merged commit c7b99cd into master Nov 11, 2016
@gavofyork gavofyork added A8-looksgood 🦄 Pull request is reviewed well. and removed A0-pleasereview 🤓 Pull request needs code review. labels Nov 11, 2016
@arkpar arkpar deleted the jg-api-connected branch November 17, 2016 08:12
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
A8-looksgood 🦄 Pull request is reviewed well. M6-rpcapi 📣 RPC API.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants