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

[Bug]: Recommended way of packaging stream schemas as JSON files should use importlib.resources #875

Closed
edgarrmondragon opened this issue Aug 1, 2022 · 3 comments
Labels

Comments

@edgarrmondragon
Copy link
Collaborator

edgarrmondragon commented Aug 1, 2022

Singer SDK Version

latest

Python Version

NA

Bug scope

Taps (catalog, state, stream maps, etc.)

Operating System

Any

Description

The currently recommended way (as used in the cookiecutter templates) to ship JSON files for the stream schemas relies on __file__, which doesn't play well with modern tools for packaging a Python application (such as a Singer tap/target) as a standalone binary, such as PyOxidizer.

The solution is to use importlib.resources or the backport package importlib_resources on Python < 3.9.

This would also require changing the following:

  1. drop support for raw string file path
  2. discourage use of Path instances
  3. change our type hints to use importlib.resources.abc.Traversable
  4. add an __init__.py to the schemas/ directory shipped with the cookiecutter

For a example, see edgarrmondragon/tap-bitso#125. The PR is currently failing static type checks because type annotations are not compatible with the latest SDK.

Code

+ from my_tap import schemas  # assumes a __init__.py exists in my_tap/schemas/
+ 
+ if sys.version_info >= (3, 9):
+     import importlib.resources as resources
+ else:
+     import importlib_resources as resources

- SCHEMAS_DIR = Path(__file__).parent / "./schemas"
+ SCHEMAS_DIR = resources.files(schemas)
@edgarrmondragon edgarrmondragon added kind/Bug Something isn't working valuestream/SDK labels Aug 1, 2022
@edgarrmondragon edgarrmondragon changed the title [Bug]: Recommended way of packaging stream should use importlib.resources [Bug]: Recommended way of packaging stream schemas as JSON files should use importlib.resources Aug 1, 2022
@edgarrmondragon edgarrmondragon linked a pull request Aug 4, 2022 that will close this issue
@stale
Copy link

stale bot commented Jul 18, 2023

This has been marked as stale because it is unassigned, and has not had recent activity. It will be closed after 21 days if no further activity occurs. If this should never go stale, please add the evergreen label, or request that it be added.

@stale stale bot added the stale label Jul 18, 2023
@edgarrmondragon
Copy link
Collaborator Author

Still relevant

@stale stale bot removed the stale label Jul 18, 2023
@edgarrmondragon
Copy link
Collaborator Author

Closed by #2130

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging a pull request may close this issue.

1 participant