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

Update structure, some names and imports #59

Closed
wants to merge 53 commits into from

Conversation

andrii-balitskyi
Copy link
Collaborator

@andrii-balitskyi andrii-balitskyi commented May 17, 2024

Closes #54 and #55

@andrii-balitskyi
Copy link
Collaborator Author

@razor-x We have a circular import error with these changes. How do you think we should restructure the code to fix this?
image

@razor-x
Copy link
Contributor

razor-x commented May 17, 2024

I don't know if this will fix things, but ideally the tests only need to import from the public API, e.g., instead of from seam.exceptions import SeamApiException it should be from seam import SeamApiException.

Meaning, importing anything like seam.* is NOT supported for consumers of this lib even if python allows it (can we prevent that usage?).

I understand if some unit tests need to import things that are not exposed on seam, however I can't think of any cases like that. And in those cases, we can write the test files alongside the module.

seam/exceptions.py Outdated Show resolved Hide resolved
@andrii-balitskyi
Copy link
Collaborator Author

andrii-balitskyi commented May 20, 2024

@razor-x I fixed SeamApiException imports in tests but circular import issue didn't go away. The issue is that seam/models.py imports AbstractRoutes and Workspace from seam.routes.models then seam/routes/__init__.py imports AccessCodes from seam.routes.access_codes and seam/routes/access_codes.py attempts to import AbstractSeam from seam.models, which is already being loaded, hence the circular import error.

@andrii-balitskyi
Copy link
Collaborator Author

Closing in favour of #63

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.

Update imports, rename files
3 participants