Skip to content
This repository has been archived by the owner on Dec 10, 2018. It is now read-only.

重构下ThriftImporter #1

Closed
wants to merge 2 commits into from

Conversation

wooparadog
Copy link
Member

  1. 之前的Importer实现没有遵守协议
  2. 其实标准库里面有import hook(ihooks)这种东西来处理非py文件. 路径查找神马的苦活累活这个东西负责了.
  3. 单拆文件
  4. 其实不应该默认加载上 import hook, 这个权限应该下放到使用者.
  5. 既然目的就是把thrift当成py module用, 那就不需要再在文件名里面加上 "_thrift" 这货了.
  6. 别合, 这个先只是FYI, 火车上写的, 只是测过了 import, 测试啊什么的都没跑.
  7. nose这货老了, 换 py.test 吧..
  8. 这种带tests的可以加上travis

好了..我收拾下睡觉去了...

@lxyu
Copy link
Contributor

lxyu commented Jun 3, 2014

哈哈 cool。

btw, 关于 import hook 这块。'_thrift' 我觉得还是要的,这个地方直接 import thrift 文件主要是提供方便,但是还是应该和标准的 py module 做一些区分,也避免了可能会出现的命名冲突。比如 'import ping' 的话,我可能真以为会有个 py package 就叫 ping 呢 =,=

然后这个 hook 要不要自动化 enable,这个和上面其实是一体的,如果有 '_thrift' 这个 suffix 作区分的话,其实是可以直接自动的 enable 掉的。反正没啥影响。

这里其实参考了 pycapnp 的处理方式,可以也参照的看一下~

@@ -0,0 +1 @@
1. Use Servers/Clients/transport code from thrift upstream, this project should only contain codes concerning protocol and magic importer
Copy link
Contributor

Choose a reason for hiding this comment

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

这个是我还在考虑的一个东西。不过基本还是倾向于抛掉 upstream 的,只作兼容处理。

  1. upstream 是 py2 only
  2. upstream 的 server/client/transport 的 code 风格都略不给力,以及难以完全满足目前的简化的 sdk 定制需求。
  3. upstream 的功能实现都太 easy 了。。所有的地方实现都非常的 raw 和基础,这个是之后 thriftpy 可以优化掉的。

初步是兼容,之后会有“高级”协议,transport (for example 0mq),etc。

Copy link
Member Author

Choose a reason for hiding this comment

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

火车上没网, 不知道thrift的代码怎么样子. 不过这样和upstream脱开真的大丈夫么..

  1. py2 only => 这个其实...嗯, 我们还会相当长时间在py2的世界里
  2. 同意, 不过可以用其他的方式来重写, 继承/mixin/delegate 啥的. 完全单开有点over kill
  3. 同2, 没看到原来的...

期待高级货..

Copy link
Contributor

Choose a reason for hiding this comment

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

哈哈其实饿了么的新项目已经都是 py3 了。。lol

and,其实如果看一下 upstream 的实现,就会发现还是非常简单的。主要是实现的种类多,而不是内容多。其本质还是在于 thrift 是一个非常简单的东西 =,=

高级协议是接下来要搞的,简单说就是 meta data,方便我们记录和追踪 service 调用的 stack 和 路径。好在 binary 里面加了个 version id,所以还是能兼容的。

transport 其实换成 zeromq 估计就好很多。server 可以融入 gevent / pulsar 等异步支持 etc。还有更多的定制化的东西可以搞。。比如甚至可以和 capnproto 做个融合之类的 想象空间还是蛮大的。。

Copy link
Member Author

Choose a reason for hiding this comment

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

已经开始用 py3 了? 👏

Copy link
Contributor

Choose a reason for hiding this comment

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

yep,除了 zeus 和一些被主要依赖锁死的,比如 (saltstack)。新项目都是 py3 或者 py3 & py2 了。

@wooparadog
Copy link
Member Author

  1. btw, 关于 import hook 这块。'_thrift' 我觉得还是要的,这个地方直接 import thrift 文件主要是提供方便,但是还是应该和标准的 py module 做一些区分,也避免了可能会出现的命名冲突。比如 'import ping' 的话,我可能真以为会有个 py package 就叫 ping 呢 =,=

从用法和语义上, 这个thrift的确是当成一个package来使用的, 至于是不是python package其实不重要. import 还能import so/c扩展呢.

  1. 然后这个 hook 要不要自动化 enable,这个和上面其实是一体的,如果有 '_thrift' 这个 suffix 作区分的话,其实是可以直接自动的 enable 掉的。反正没啥影响。

我感觉如果非显示要求, 还是尽量不要去修改运行时, 这个是可能让某些人的东西break掉的.


综合1&2, 为了方便提供import的机制是非常great的, 但是对于使用和带来的副作用这些心智负担还是给用户来承担比较好.

@lxyu
Copy link
Contributor

lxyu commented Jun 3, 2014

对于 1 我觉得还是有区别的。so/c extension 其实都是已经被大家接受的东西,使用比较广泛,这个 '_thrift' 类型的 hack 我觉得还是显式的出来会更好。

对于 install 这个,我觉得无所谓的,两种方式其实都差不多,比如 pycapnp 之前是手动 install,后来改为自动 install 但是提供一个 remove 的接口。自动 install 的话,如果是 "_thrift" namespace 下,其实是不会 break 掉其他地方的东西的 =,=

capnp 和 flask ext hack 的做法,FYI

https://github.com/jparyani/pycapnp/blob/develop/capnp/lib/capnp.pyx#L3059

https://github.com/mitsuhiko/flask/blob/master/flask/ext/__init__.py

@wooparadog
Copy link
Member Author

嗯, 其实这两种都差不多..

  1. 默认打开 importer, 然后import 的时候显示标注_thrift
  2. 默认关闭 importer, 然后手动打开后把 thrift 当做 module 看待

依然觉得后一种好些. 😆

没详细看capnp, 不过它是因为用到这个包就需要import那个东西, 所以默认import吧?
flask那个.. We're switching from namespace packages because it was just too painful for everybody involved. 😆

@lxyu
Copy link
Contributor

lxyu commented Jun 3, 2014

flask 里面提到的 namespace 的概念应该和我们上文讨论中的 namespace 概念不一样。这里 complain 的正是这个 hook 所改进的东西。

之前应该是需要 "from flask.ext.foo import bar"。可以想象写一个正常 work 的 foo extension,要搞定这个前面的 "flask.ext" path 多麻烦。

@lxyu
Copy link
Contributor

lxyu commented Jun 3, 2014

pycapnp 和我们这里的情况是基本一致的,它用到的是 'pingpong.capnp',这里用的是 ’pingpong.thrift‘

@wooparadog
Copy link
Member Author

是觉得flask他们为了namespace好看, 真心做了好多事情.

跑去看了下PEP 302, ihooks原来是个老用法.. 没网真心不能做开发.. close了.

@wooparadog wooparadog closed this Jun 3, 2014
ethe pushed a commit to ethe/thriftpy that referenced this pull request Dec 8, 2018
Make toro only required with tornado 4
ethe added a commit to ethe/thriftpy that referenced this pull request Dec 8, 2018
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants