singer-io / singer-tap-template

GNU Affero General Public License v3.0
68 stars 31 forks source link

Workaround for non installed execution #2

Closed meteozond closed 6 years ago

meteozond commented 6 years ago

This will make possible debugging and running taps without installing them including local imports situations. Like this:

python -m example_tap

dmosorast commented 6 years ago

@meteozond I'm not sure I understand this pattern, since, as far as I know, the dependencies would still need to be present on the system in order to run it from the source directly without pip install.

Also, the template should be providing this entry point for running python -m tap_name with this code at the bottom of __init__.py.

if __name__ == "__main__":
    main()

Testing this by running tap_zendesk shows that it will run the module, but fail on dependencies.

$ python3 -m tap_zendesk
Traceback (most recent call last):
  File "/usr/lib/python3.5/runpy.py", line 174, in _run_module_as_main
    mod_name, mod_spec, code = _get_module_details(mod_name, _Error)
  File "/usr/lib/python3.5/runpy.py", line 133, in _get_module_details
    return _get_module_details(pkg_main_name, error)
  File "/usr/lib/python3.5/runpy.py", line 109, in _get_module_details
    __import__(pkg_name)
  File "/opt/code/tap-zendesk/tap_zendesk/__init__.py", line 4, in <module>
    from zenpy import Zenpy
ImportError: No module named 'zenpy'

Can you provide some more information/documentation to describe this pattern?

meteozond commented 6 years ago

@dmosorast As for me, development case mean that developer knows about possible dependencies, but if you intend on explicit approach to requirements:

  1. requirements.txt could be taken out from setup.py and replaced with reading this file code.
  2. readme could be extended with development section, including instructions for contributors.

About main() execution:

  1. It is already present in __main__.py so it is enough to execute module during development.

  2. Setup.py Entrypoint is replaced with __main__:main so enduser won't see any changes.

The main problem today that developing new complicated tap/target requires running setup on each test/debug execution.

timvisher commented 6 years ago

@meteozond If I understand correctly, you're trying to avoid setting up a virtualenv via python3 -m venv <virtualenv and making your development life easier by not having to pip install . every time you change your source code.

As far as not setting up a virtualenv for development, that's a non-starter. virtualenvs are central to Singer and, as far as I know, basically a standard requirement for production Python deployments.

That said, you should not need to run pip install . over and over again during development. All you need to do is instead run pip install -e . once, and all your edits will be available each time you run tap-foo.

There's nothing inherently wrong with wanting to run python -m tap_foo but there's also nothing inherently wrong with wanting to install it into a virtualenv and have init as the entry point, so for the sake of consistency we'll keep it as that.

However, you're absolutely right about the README needing a development section. It should read something like:

## Development

1. Create a virtualenv
1. Run `pip install -e '.[dev]'`
1. Run `tap-foo`

To complete that change, we'd also want an extras_require section in setup.py like this, probably including exactly those lines since you almost always want pylint and ipdb available in dev.

meteozond commented 6 years ago

@timvisher, @dmosorast I've got your idea, but I don't like it because of a lot of magic behind it. Packaging, unpacking, coping the code result complication and possible side effects from tools involved. It can take a lot of time is some cases. My approach lets you to run code inplace in preinstalled environment.

timvisher commented 6 years ago

@meteozond Understood. Thanks for going back an forth for a bit.

I reified our discussion into https://github.com/singer-io/singer-tap-template/issues/3