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

Add submodule to sys.path when loading child idl file. #249

Merged
merged 2 commits into from
Mar 13, 2024

Conversation

cocolato
Copy link
Contributor

@cocolato cocolato commented Mar 8, 2024

Fixes: #215

I'm not sure if it's suitable to add the module directly to sys.path, maybe is should implement like this:

for module in thrift.__thrift_meta__["includes"]:
    if module not in sys.modules:
        sys.modules[module.__name__+"_thrift"] = module

then use the import hook to make sure pickle will import the suitable module.

@cocolato cocolato changed the title add submodule to sys.path when loading module Add submodule to sys.path when loading child idl file. Mar 8, 2024
thriftpy2/parser/__init__.py Outdated Show resolved Hide resolved
@cocolato cocolato requested a review from aisk March 11, 2024 02:09
@cocolato
Copy link
Contributor Author

Now has been updated to a recursive implementation.

@aisk
Copy link
Member

aisk commented Mar 11, 2024

I have some concern about whether this code can handle circle including IDLs. Can you add a test case to ensure this?

@cocolato
Copy link
Contributor Author

Ok, i'll test this part later.

@cocolato
Copy link
Contributor Author

I have some concern about whether this code can handle circle including IDLs. Can you add a test case to ensure this?

There are already have tests for dead inclusions.

def test_e_dead_include():
with pytest.raises(ThriftParserError) as excinfo:
load('parser-cases/e_dead_include_0.thrift')
assert 'Dead including' in str(excinfo.value)

@aisk aisk merged commit e0f0081 into Thriftpy:master Mar 13, 2024
8 checks passed
@cocolato cocolato deleted the fix/pickle_submodule branch March 14, 2024 09:49
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.

cannot pickle dump object from child idl file class
3 participants