uber / tchannel

network multiplexing and framing protocol for RPC
MIT License
1.15k stars 129 forks source link

update the thrift meta file for thriftIDL #1335

Closed ShanniLi closed 8 years ago

ShanniLi commented 8 years ago

r @prashantv @Raynos @junchaowu cc @blampe

@Raynos suggested us to move forward implementing a default handler for thriftIDL ... Node example is here: #1332

prashantv commented 8 years ago

As I mentioned earlier, can we please think about the interface that is appropriate here

Raynos commented 8 years ago

We can always have oneThriftIdl and manyThriftIdl

Or we can fix it right now.

prashantv commented 8 years ago

Let's fix it now, it's not hard to make a new struct with a map.

Raynos commented 8 years ago

Cool let's fix it.

ShanniLi commented 8 years ago

Meta is implemented per service, right?

Raynos commented 8 years ago

The map is to future proof for import statements.

prashantv commented 8 years ago

There is a single Meta service for each Hyperbahn service. One hyperbahn service can expose multiple Thrift services though.

We also may have multiple Thrift files due to imports.

ShanniLi commented 8 years ago

I see. Makes sense.

ShanniLi commented 8 years ago

So, something like the following? struct ThriftIDLs { 1: required map<string,string> idls }

ThriftIDLs thriftIDL()

prashantv commented 8 years ago

yep, I think the map should be map<string, string> idls, but I think that's good enough for now

And we should add a comment for the map explaining what idls is (map from filename -> contents)

Raynos commented 8 years ago

@ShanniLi

We need also 2: required entryPoint to tell tcurl which file is the root that imports the others. Otherwise hacks

ShanniLi commented 8 years ago

@Raynos fixed.

prashantv commented 8 years ago

lgtm