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

Using Pydantic creates circular imports #598

Open
3 tasks done
AdrienVannson opened this issue Aug 16, 2024 · 5 comments
Open
3 tasks done

Using Pydantic creates circular imports #598

AdrienVannson opened this issue Aug 16, 2024 · 5 comments
Labels
compiler-bug The compiler is broken pydantic Issues with pydantic integration

Comments

@AdrienVannson
Copy link
Contributor

AdrienVannson commented Aug 16, 2024

Summary

Using Pydantic creates circular imports

Reproduction Steps

Compile the following .proto files:

A_a.proto:

syntax = "proto3";
package root.apackage;

import "B.proto";

message AaMsg {
    root.bpackage.BMsg x = 1;
}

A_b.proto:

syntax = "proto3";
package root.apackage;

message AbMsg {
    int32 x = 1;
}

B.proto:

syntax = "proto3";
package root.bpackage;

import "A_b.proto";

message BMsg {
    apackage.AbMsg x = 1;
}

There is no circular dependency between the .proto files. However, after the compilation, there is a circular dependency between the two packages.

Expected Results

I compiled the .proto files with the pydantic_dataclasses.
I would expect importing the library with import root.apackage to work correctly.

Actual Results

import root.apackage fails with the following error:

Traceback (most recent call last):
  File "<stdin>", line 1, in <module>
  File "XXX/root/apackage/__init__.py", line 17, in <module>
    from .. import bpackage as _bpackage__
  File "XXX/root/bpackage/__init__.py", line 21, in <module>
    class BMsg(betterproto.Message):
  File "XXX/.venv/lib/python3.10/site-packages/pydantic/dataclasses.py", line 249, in create_dataclass
    pydantic_complete = _pydantic_dataclasses.complete_dataclass(
  File "XXX/.venv/lib/python3.10/site-packages/pydantic/_internal/_dataclasses.py", line 120, in complete_dataclass
    set_dataclass_fields(cls, types_namespace, config_wrapper=config_wrapper)
  File "XXX/.venv/lib/python3.10/site-packages/pydantic/_internal/_dataclasses.py", line 82, in set_dataclass_fields
    fields = collect_dataclass_fields(cls, types_namespace, typevars_map=typevars_map, config_wrapper=config_wrapper)
  File "XXX/.venv/lib/python3.10/site-packages/pydantic/_internal/_fields.py", line 313, in collect_dataclass_fields
    ann_type = _typing_extra.eval_type_lenient(dataclass_field.type, types_namespace, cls_localns)
  File "XXX/.venv/lib/python3.10/site-packages/pydantic/_internal/_typing_extra.py", line 240, in eval_type_lenient
    return eval_type_backport(value, globalns, localns)
  File "XXX/.venv/lib/python3.10/site-packages/pydantic/_internal/_typing_extra.py", line 264, in eval_type_backport
    return typing._eval_type(  # type: ignore
  File "/usr/lib/python3.10/typing.py", line 327, in _eval_type
    return t._evaluate(globalns, localns, recursive_guard)
  File "/usr/lib/python3.10/typing.py", line 694, in _evaluate
    eval(self.__forward_code__, globalns, localns),
  File "<string>", line 1, in <module>
AttributeError: partially initialized module 'pygrpc2_issue.root.apackage' has no attribute 'AbMsg' (most likely due to a circular import)

System Information

libprotoc 3.12.4
Python 3.10.12
Name: betterproto
Version: 2.0.0b6
Summary: A better Protobuf / gRPC generator & library
Home-page: https://github.com/danielgtaylor/python-betterproto
Author: Daniel G. Taylor
Author-email: [email protected]
License: MIT
Location: XXX/.venv/lib/python3.10/site-packages
Requires: grpclib, python-dateutil, typing-extensions
Required-by: pygrpc-poc

Checklist

  • I have searched the issues for duplicates.
  • I have shown the entire traceback, if possible.
  • I have verified this issue occurs on the latest prelease of betterproto which can be installed using pip install -U --pre betterproto, if possible.

Notes

When Pydantic is not used, the circular dependency is handled correctly by Python.

@AdrienVannson AdrienVannson added bug Something isn't working investigation needed labels Aug 16, 2024
@Gobot1234 Gobot1234 added compiler-bug The compiler is broken pydantic Issues with pydantic integration and removed bug Something isn't working investigation needed labels Aug 16, 2024
@AdrienVannson
Copy link
Contributor Author

I think that the problem can be solved by moving some imports to the end of the generated files, just before all the rebuild_dataclass. Before I try to do it and see if it actually work as expected, I have a question: is it something that we actually want to do?

Or should we consider that circular package dependencies are not acceptable and should indeed produce an error? As far as I know, they work correctly with the original gRPC python client and betterproto without pydantic, but I'm not sure about the other clients.

@Gobot1234
Copy link
Collaborator

There's no reason for them not to work. Please if you do fix this add a test so this doesn't happen in the future.

@AdrienVannson
Copy link
Contributor Author

AdrienVannson commented Aug 19, 2024

Great, thanks! Actually, it seems that there is already a test for that: tests/inputs/import_circular_dependency. However, this test is only executed without pydantic, so no error is shown.

If I replace plugin_output_package = "tests.output_betterproto" by plugin_output_package = "tests.output_betterproto_pydantic" in test_inputs.py, I have the expected error... but a lot of other unrelated tests fail as well.

@AdrienVannson
Copy link
Contributor Author

AdrienVannson commented Aug 19, 2024

Actually, I have the impression that it is not that simple to fix. I tried putting the imports from get_type_reference at the end of the file, as I said. However, in the test import_circular_dependency, in the line of the generated file: other: "other.OtherPackageMessage" = betterproto.message_field(2), I am not sure why, but there seem to be a problem due to the fact that the name other is used twice.

After I fixed that quickly by adding a prefix, in my project, I still had errors occurring during the calls to rebuild_dataclass. As I'm not sure how long it would take me to make everything work and how many changes to the project will be needed, I think I'll stop here for now

@AdrienVannson
Copy link
Contributor Author

In the end, I probably have a solution. I will test it, and if it works, I will send a PR once the other issues that I opened are fixed (I need the fix to solve this problem)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
compiler-bug The compiler is broken pydantic Issues with pydantic integration
Projects
None yet
Development

No branches or pull requests

2 participants