exceptions defined in idls and included by other idl does not work if thrift file loaded by import hook

[shuoli84]

two idls:

common.thrift:

exception CommonEx {
    1 i32 code,
}

app.thrift

include "common.thrift"

service AppService {
    i64 do_something() (1: common.CommonEx ex)
}

if import them by import hook

from common_thrift import CommonEx   # CommonExA
from app_thrift import AppService  # The CommonEx in AppService as CommonExB

# this assert will fail.
assert isinstance(CommonEx(code=1), AppService.do_something_result.ex) 

Reason: CommonExA's module is "common_thrift", CommonExB's module is "common"

[shuoli84]

The problem is

import app_thrift
import common_thrift

common_thrift != app_thrift.common

[ethe]

I do not think it is a "bug", always use app_thrift.common is fine I think. However, this behavior is not in the Thrift spec so there is not a standard about it.

[shuoli84]

Yes, I also think this is not defined in spec, so this leaves detail to implementation. Just believe current behavior is counter intuitive.

For a normal python class, following always holds true.

from package_a import module_b
from package_a.module_b import ClassA

assert modle_b.ClassA is ClassA

[shuoli84]

The above comment seems flawed, but you get my point. :)

Is it something we can do to fix it? Or warn users?

[shuoli84]

And also, if we use gen code, the problem gone. This only happens in thriftpy. Maybe we can track all dynamic generated modules and use the same module object for same file?

[microdog]

@shuoli84 I assume that you use two load calls to load the app_thrift and common_thrift modules.

The parsed module is cached in thriftpy by default, and the file path is used as the cache key. If you do not specify module_name, you will get the result you expected.

app_thrift = thriftpy2.load('app.thrift')
common_thrift = thriftpy2.load('common.thrift')

assert app_thrift.common is common_thrift  # True

The problem is that when the included file is parsed, the default module_name will be used, and currently, it will be the file path. When you load the included file using a different module_name, which will be used as the cache key, the cache will miss and a new module will be generated.

[shuoli84]

@microdog Thanks for the comment, you are right. I import the thrift by import hook, which use filename_thrift as its default module name, and nested modules use different names, that caused the problem.

Any idea on how to fix this? I presumed import hook has same behavior as plain load. Seems we just need to figure out a way to ensure same file always has same cache key?

[microdog]

@shuoli84 There is a simple hack if you really want to import the included file:

import sys

import app_thrift

sys.modules['common_thrift'] = app_thrift.common

[shuoli84]

@microdog I already solved my problem, just use the app_thrift.common in code. So the hack is not needed. The reason why I still asking whether there is a fix is, maybe other people didn't aware of this and then have to waste time in troubleshoot.