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.

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

R4R: Fix CLI commands JSON output #2266

Merged
merged 2 commits into from
Sep 8, 2018
Merged

Conversation

alessio
Copy link
Contributor

@alessio alessio commented Sep 7, 2018

When running with --json, commands should produce
correctly JSON-encoded output.

Closes: #2265

  • Linked to github-issue with discussion and accepted design OR link to spec that describes this work.
  • Wrote tests
  • Updated relevant documentation (docs/)
  • Added entries in PENDING.md with issue #
  • rereviewed Files changed in the github PR explorer

For Admin Use:

  • Added appropriate labels to PR (ex. wip, ready-for-review, docs)
  • Reviewers Assigned
  • Squashed all commits, uses message "Merge pull request #XYZ: [title]" (coding standards)

@alessio alessio changed the title Fix CLI commands JSON output R4R: Fix CLI commands JSON output Sep 7, 2018
@alexanderbez
Copy link
Contributor

@alessio can you post example output please?

@alessio
Copy link
Contributor Author

alessio commented Sep 7, 2018

@alexanderbez sure:

Broken encoding:

{"Height":"4","TxHash":"30767493550CC1601F429A9A7D795DB0E5A72121","Response":"{Code:0 Data:[] Log:Msg 0:  Info: GasWanted:3107 GasUsed:3107 Tags:[{Key:[115 101 110 100 101 114] Value:[99 111 115 109 111 115 49 100 110 57 51 119 100 106 55 51 103 106 107 48 120 101 110 51 53 109 116 109 56 99 116 104 107 102 104 99 101 108 106 104 97 110 116 109 118]} {Key:[114 101 99 105 112 105 101 110 116] Value:[99 111 115 109 111 115 49 115 114 100 52 116 52 101 55 97 120 119 120 115 113 120 114 117 107 53 102 109 51 99 102 113 101 108 106 97 51 122 112 119 121 117 118 110 119]}] XXX_NoUnkeyedLiteral:{} XXX_unrecognized:[] XXX_sizecache:0}"}

Correct encoding:

{"Height":"6","TxHash":"AAFE31D0BC9AF51C483437EAA0126F22596FB130","Response":{"log":"Msg 0: ","gas_wanted":"3107","gas_used":"3107","tags":[{"key":"c2VuZGVy","value":"Y29zbW9zMWFkbWdjZnE3ZmZhNGRxOTBlamh1MjJjYTIyemdzdHR3eHduNnRh"},{"key":"cmVjaXBpZW50","value":"Y29zbW9zMWhrZTJ5bDY1MDN2MDB6MmdsdXVzZWRyMzR0dm15Z2ZtdTJ4YTBz"}]}}

@alessio
Copy link
Contributor Author

alessio commented Sep 7, 2018

Attaching here ResponseDeliverTx's definition for convenience:

type ResponseDeliverTx struct {
	Code                 uint32          `protobuf:"varint,1,opt,name=code,proto3" json:"code,omitempty"`
	Data                 []byte          `protobuf:"bytes,2,opt,name=data,proto3" json:"data,omitempty"`
	Log                  string          `protobuf:"bytes,3,opt,name=log,proto3" json:"log,omitempty"`
	Info                 string          `protobuf:"bytes,4,opt,name=info,proto3" json:"info,omitempty"`
	GasWanted            int64           `protobuf:"varint,5,opt,name=gas_wanted,json=gasWanted,proto3" json:"gas_wanted,omitempty"`
	GasUsed              int64           `protobuf:"varint,6,opt,name=gas_used,json=gasUsed,proto3" json:"gas_used,omitempty"`
	Tags                 []common.KVPair `protobuf:"bytes,7,rep,name=tags" json:"tags,omitempty"`
	XXX_NoUnkeyedLiteral struct{}        `json:"-"`
	XXX_unrecognized     []byte          `json:"-"`
	XXX_sizecache        int32           `json:"-"`
}

@alexanderbez
Copy link
Contributor

alexanderbez commented Sep 7, 2018

Thank you -- unfortunate the key/values are not decoded :-/

Copy link
Contributor

@alexanderbez alexanderbez left a comment

Choose a reason for hiding this comment

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

utACK 👍

When running with --json, commands should produce
correctly JSON-encoded output.

Closes: #2265
@codecov
Copy link

codecov bot commented Sep 8, 2018

Codecov Report

Merging #2266 into develop will not change coverage.
The diff coverage is n/a.

@@           Coverage Diff            @@
##           develop    #2266   +/-   ##
========================================
  Coverage    63.73%   63.73%           
========================================
  Files          140      140           
  Lines         8642     8642           
========================================
  Hits          5508     5508           
  Misses        2755     2755           
  Partials       379      379

@alessio alessio mentioned this pull request Sep 8, 2018
5 tasks
@rigelrozanski
Copy link
Contributor

lint fail

@alessio
Copy link
Contributor Author

alessio commented Sep 8, 2018

@rigelrozanski the failure seems unrelated

@alessio
Copy link
Contributor Author

alessio commented Sep 8, 2018

@rigelrozanski the lint failure seems to have been introduced with 753e58b - @ValarDragon any thoughts?

@ValarDragon
Copy link
Contributor

Yeah, basically Jeremy's IAVL pr magically made the linter catch more errors. I have this particular lint failure fixed in #2273

Copy link
Contributor

@cwgoes cwgoes left a comment

Choose a reason for hiding this comment

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

utACK

@cwgoes cwgoes merged commit f68e5a7 into develop Sep 8, 2018
@cwgoes cwgoes deleted the alessio/2265-cli-json-output branch September 8, 2018 08:33
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants