Skip to content

Add pbToJson function #251

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

Closed
wants to merge 4 commits into from
Closed

Add pbToJson function #251

wants to merge 4 commits into from

Conversation

pawanrawal
Copy link
Contributor

@pawanrawal pawanrawal commented Sep 29, 2016

Attempt to add a function which can convert the protocol buffer response to a JSON response. This would avoid maintaining both ToJSON and ToProtocolBuffer.


This change is Reviewable

@pawanrawal pawanrawal added the kind/feature Something completely new we should consider. label Sep 29, 2016
m = make(map[string]interface{})
// This method converts the response from ToProtocolBuffer to an intermediate
// data structure that can be marshalled to JSON.
func pbToJson(gr []*graph.Node) map[string][]interface{} {

Choose a reason for hiding this comment

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

func pbToJson should be pbToJSON

@coveralls
Copy link

Coverage Status

Coverage decreased (-2.6%) to 56.189% when pulling 2cbba7f on feature/pbtojson into aaba7d7 on master.

@coveralls
Copy link

Coverage Status

Coverage decreased (-2.6%) to 56.189% when pulling 2cbba7f on feature/pbtojson into aaba7d7 on master.

@coveralls
Copy link

Coverage Status

Coverage decreased (-2.5%) to 56.275% when pulling afc5d56 on feature/pbtojson into aaba7d7 on master.

@@ -971,6 +980,38 @@ func BenchmarkToJSON_100_Director(b *testing.B) { benchmarkToJson("benchmark/di
func BenchmarkToJSON_1000_Actor(b *testing.B) { benchmarkToJson("benchmark/actors1000.bin", b) }
func BenchmarkToJSON_1000_Director(b *testing.B) { benchmarkToJson("benchmark/directors1000.bin", b) }

func benchmarkToJson2(file string, b *testing.B) {

Choose a reason for hiding this comment

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

func benchmarkToJson2 should be benchmarkToJSON2


// This method converts the response from ToProtocolBuffer to an intermediate
// data structure that can be marshalled to JSON.
func pbToJson(gr []*graph.Node) map[string][]interface{} {

Choose a reason for hiding this comment

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

func pbToJson should be pbToJSON

@@ -386,6 +386,43 @@ func (sg *SubGraph) ToJSON(l *Latency) ([]byte, error) {
return nil, fmt.Errorf("Runtime should never reach here.")
}

func (sg *SubGraph) ToJSON2(l *Latency) ([]byte, error) {

Choose a reason for hiding this comment

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

exported method SubGraph.ToJSON2 should have comment or be unexported

@coveralls
Copy link

Coverage Status

Coverage decreased (-1.4%) to 57.397% when pulling 2527d92 on feature/pbtojson into aaba7d7 on master.

@pawanrawal pawanrawal changed the title WIP - Add pbToJson function Add pbToJson function Sep 29, 2016
@pawanrawal pawanrawal added improvement and removed kind/feature Something completely new we should consider. labels Sep 29, 2016
@pawanrawal
Copy link
Contributor Author

After this change the benchmarks for ToJSON aren't that good https://discuss.dgraph.io/t/adding-a-pbtojson-function/938. Hence decided not to go with this for now.

@pawanrawal pawanrawal closed this Oct 6, 2016
@pawanrawal pawanrawal deleted the feature/pbtojson branch November 28, 2016 08:16
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

4 participants