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

boilerplate codegen first cut #75

Closed
wants to merge 82 commits into from
Closed

boilerplate codegen first cut #75

wants to merge 82 commits into from

Conversation

sasha-s
Copy link

@sasha-s sasha-s commented Jul 10, 2015

#70

Review on Reviewable

@sasha-s
Copy link
Author

sasha-s commented Jul 12, 2015

How do we move forward with this?
there are few missing pieces: client code, makeEndpoint and thrift/grpc stuff.
the latter is not clear - we either generate the whole shebang or we need to know how fields/accessors are named. Client code and makeEndppint are pretty straitforward.

@peterbourgon
Copy link
Member

Thrift/gRPC/etc. will have as a dependency the IDL, and (optionally) the generated .go files. Or, put another way, we won't attempt to generate either of those. Does that help?

)
go func() {
var response ResponseT
if err := c.Call("addsvc.Add", request, &response); err != nil {
Copy link
Author

Choose a reason for hiding this comment

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

addsvc.Add should not be here.

@sasha-s
Copy link
Author

sasha-s commented Jul 13, 2015

Thrift/gRPC/etc. will have as a dependency the IDL, and (optionally) the generated .go files. Or, put another way, we won't attempt to generate either of those. Does that help?

OK.
Now, in addsvc/grpc_binding.go we have
r, err := b.Endpoint(ctx, reqrep.AddRequest{A: req.A, B: req.B})
We need to know the names in a protobuffer (A, B in this case).
Similar stuff in Thrift case (both binding and client side).
We can either rely on them being parallel to the names in the interface or get the mapping somewhere.

@peterbourgon
Copy link
Member

Cool! This got very comprehensive very quickly :) I think I want to clean up the examples, not just your addsvc2 but my old addsvc as well, they are getting rather complicated.

@sasha-s
Copy link
Author

sasha-s commented Jul 14, 2015

GRPC/thrift: maybe we can have a binding is something like

type thriftBinding struct {
    Ctx context.Context
    endpoint.Endpoint
    Upconvert, Downconvert func(interface{}) (interface{}, error)
}

then generating code is trivial (we still need to know the name for thriftadd.AddReply and request).
We can make a default implementation that uses reflection (assuming the fields are named in the same way).
If we do that, the user of the codegen has a lot of control.

The alternative is to say that the names should match and we generate the upconvert/downconvert automatically .

@peterbourgon
Copy link
Member

Forgive the delay, I've been on holiday.

This is a great prototype implementation to prove it can be done. To get merged, a couple things need to happen:

  • Test coverage. We should target 100% and document the places where that's not feasible, for whatever reason.
  • Refactoring. This has a very prototype feel, which is fine for a first draft, but it's not going to be maintainable. In the end the implementation of the generator will be the same as any other part of Go kit, carefully considered and optimized. It's difficult to say what this will entail until we get tests in place and can make changes without fear of breaking things.
  • Examples. addsvc2 was fine for prototyping but it needs to be cleaned up. Either we integrate the generator into addsvc, or create a new example specifically for the generator subdirectory.

I can contribute to refactoring and examples, but only after test coverage is established, and I think you're best qualified to tackle that. What do you think?

@sasha-s
Copy link
Author

sasha-s commented Jul 28, 2015

Hope you had a nice vacation Peter.

Sure, it all makes sense.
I will start adding high-level tests.

@ernesto-jimenez
Copy link

@sasha-s, @peterbourgon Go 1.5 added go/types to the stdlib.

If you use go/types rather than go/ast it should result in less code and easier to follow.

Yesterday I did a quick implementation from scratch an alternative to github.com/vektra/mockery using go/types. The result had ~24% less code (I think the code is also simpler) while fixing some bugs for corner cases such generating mocks for stdlib packages and mocks for interfaces with composition.

If you are interested, you can check my implementation of mockery. It was a very quick implementation, so it's lacking in comments, but I would be happy to answer any questions ;)

peterbourgon and others added 23 commits August 30, 2015 12:47
…when binding log.Valuers.

The stack depth must be consistent regardless of how many times Context.With has been called
or whether Context.Log is called via an interface typed variable or a concrete typed variable.
Also improvements to transport/http.
Updated function declarations that are not compatible with the transport/http.DecodeFunc and transport/http.EncodeFunc
code examples updated.
- Better defaultErrorEncoder that can yield StatusBadRequest
- No need to close request bodies in server; it will happen anyway.
@peterbourgon
Copy link
Member

With some sadness, I am closing this PR. I would eagerly welcome a new draft, based on current master, and with some structure that is amenable to future maintenance.

@joeblew99
Copy link

there are code gen implementation i am playing with:

https://github.com/moul/protoc-gen-gotemplate

  • github.com/moul/chatsvc

https://github.com/kujtimiihoxha/gk

--

I am planning to extend the moul one to support a CQRS storage architecture for the server and the client. All storage is a noSQL store.
Design here: https://rawgit.com/gu-io/gu-x/master/design.svg

Server using minio buckets
Client using boltDB buckets

In order to support CQRQ code gen, the design uses Materialised Views that are generated based on the protofuf. A Materialised views is one computed from the base types.
For example a Patient and the Medications that a patient uses.

  • The Patients Service is a simple list of Patients (with CRUD).
  • The Medications Service is a simple list (with CRUD).
  • The PatientsMedications is a list of medications only for that patient. This is computed via the Materialised View. When any of the Medication names changes ( a mutation), the system knows that PatientsMedications needs to be updated for all "FK" (ForeignKeys).

Mutations on a base type (Medications) force all FK's to it are lookedup and then recomputed.
Mutations are stored in an Outbox bucket and Inbox bucket.

  • Outbox is for changes the client made
  • Inbox is for mutations that another client made, and that you receiving.

Conflict management of data clashes is handled with a simple OT compare.

@peterbourgon You said to contact you about this, so if you want to comment on this i would be appreciative.

@peterbourgon
Copy link
Member

@joeblew99 Did you mean to comment on a 2-year-old closed PR?

@joeblew99
Copy link

joeblew99 commented Apr 18, 2017 via email

@peterbourgon
Copy link
Member

I reckon the relevant issue is #70

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.