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

Tracedb interface && cli #997

Merged
merged 4 commits into from
May 2, 2016
Merged

Tracedb interface && cli #997

merged 4 commits into from
May 2, 2016

Conversation

debris
Copy link
Collaborator

@debris debris commented Apr 24, 2016 •
edited
Loading

this pr should be merged after:

#992 #994 #996

rpc docs

https://gist.github.com/debris/051c131e877affefeab4553509640a43

this pr includes:

  • client api for tracing
  • actual storing traces in database
  • tracing cli
  • traces rpc

@debris debris added A0-pleasereview 🤓 Pull request needs code review. A3-inprogress ⏳ Pull request is in progress. No review needed at this stage. and removed A0-pleasereview 🤓 Pull request needs code review. A3-inprogress ⏳ Pull request is in progress. No review needed at this stage. labels Apr 24, 2016
@debris debris mentioned this pull request Apr 28, 2016
Merged
@gavofyork
Copy link
Contributor

will review after #995 merged to minimise the clutter.

looking at what is generated, there's at least one typo.

@gavofyork gavofyork added A6-mustntgrumble 💦 Pull request has areas for improvement. The author need not address them before merging. and removed A0-pleasereview 🤓 Pull request needs code review. labels Apr 30, 2016
@gavofyork
Copy link
Contributor

TraceID (an identifier substring used in previous code) versus TraceId in this PR. that said, i think this is endemic in the codebase and we'll need a proper sweep to get rid of this inconsistency.

gavofyork
gavofyork reviewed Apr 30, 2016
View reviewed changes
parity/cli.rs
--tracing BOOL Indicates if full transaction tracing should be
enabled. Works only if client had been fully synced with
tracing enabled. BOOL may be one of auto, on, off.
auto uses last used value of this option (off it does
Copy link
Contributor

Choose a reason for hiding this comment

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

"off if it does"

@gavofyork gavofyork mentioned this pull request Apr 30, 2016
Closed
tomusdrw
tomusdrw reviewed Apr 30, 2016
View reviewed changes
parity/rpc.rs
@@ -115,6 +115,10 @@ pub fn setup_rpc_server(
// not adding to modules, since `ethcore` is not supported in geth
server.add_delegate(EthcoreClient::new(&deps.miner, deps.logger.clone(), deps.settings.clone()).to_delegate())
},
"traces" => {
// not adding to modules, since `traces` is not supported in geth
server.add_delegate(TracesClient::new(&deps.client).to_delegate())
Copy link
Collaborator

Choose a reason for hiding this comment

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

Should be also included in parity/webapps.rs?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I have no idea... It's not a part of 'standard' rpc, but it would be useful

Copy link
Collaborator

Choose a reason for hiding this comment

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

I think it should be added just for consistency.

@debris
Copy link
Collaborator Author

debris commented May 1, 2016 •
edited
Loading

TraceID (an identifier substring used in previous code) versus TraceId in this PR. that said, i think this is endemic in the codebase and we'll need a proper sweep to get rid of this inconsistency.

What do you mean? Which TraceID?

@gavofyork
Copy link
Contributor

Was thinking of CacheID, sorry.

@gavofyork gavofyork added A8-looksgood 🦄 Pull request is reviewed well. and removed A6-mustntgrumble 💦 Pull request has areas for improvement. The author need not address them before merging. labels May 2, 2016
@gavofyork gavofyork merged commit 7c2adc4 into master May 2, 2016
@gavofyork gavofyork deleted the tracedb_int branch May 2, 2016 10:17
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers
No reviews
Assignees
No one assigned
Labels
A8-looksgood 🦄 Pull request is reviewed well.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@debris @gavofyork @tomusdrw