-
Notifications
You must be signed in to change notification settings - Fork 4.3k
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
Python Developer Experience #684
Conversation
'base_singer', | ||
'airbyte_protocol' | ||
] | ||
"psycopg2==2.8.4", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
upgraded this because installing 2.7.4 fails locally on my local Mac/Py3.8
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
nice!
can you add into source somewhere a readme of how to get up and running with python? just synthesizing some of the stuff you mentioned. doesn't have to be long. but if i'm a new person to the team, it should be enough that i can get my second intellij instance running without asking for help.
tools/python/.isort.cfg
Outdated
[settings] | ||
# This is to make isort compatible with Black. See | ||
# https://black.readthedocs.io/en/stable/the_black_code_style.html#how-black-wraps-lines. | ||
line_length=88 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
is this kinda small?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
140 pls?
@@ -0,0 +1,46 @@ | |||
apply plugin: 'ru.vyarus.use-python' |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
nice.
tools/python/.isort.cfg
Outdated
[settings] | ||
# This is to make isort compatible with Black. See | ||
# https://black.readthedocs.io/en/stable/the_black_code_style.html#how-black-wraps-lines. | ||
line_length=88 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
140 pls?
from .source import * | ||
from .source import TemplatePythonSource | ||
|
||
__all__ = ["TemplatePythonSource"] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
For my own curiosity, why do we need that line?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
To better support introspection, modules should explicitly declare the names in their public API using the all attribute.
Moreover, it solves flake8
warnings:
https://stackoverflow.com/a/49266468
Great setup with the symlinks! I found adding an extra symlink in airbyte-integration to airbyte_protocol (maybe have to add one for singer-source too?) also allows you to open your IDE in the integration folder and navigate all connectors/templates at once quite nicely! |
This PR sets up for Gradle:
./gradlew build
, Python can be run locally, in IntelliJ, and has proper syntax highlighting in IntelliJ.pygradle
which was cumbersome to use and slow to initialize.In a separate PR we can easily add
pytest
testing (we could also set uptox
similarly).Caveats:
It's very hard to get Python working nicely in IntelliJ alongside Java.
I can get proper syntax highlighting to work while using symlinked dependencies, but it doesn't keep the module set as Python after you manually set it and then refresh Gradle unless you exclude the sources completely from the Java plugin in Gradle. Even then, you have to set a venv to work manually. What seems to work best is to have a single Python interpreter set up for the integration's venv you're working with. Then you can set all of the facets to that interpreter. If you need to switch interpreters, then you change the global interpreter. This definitely is ideal but I couldn't find any way to configure IntelliJ to autodetect and use subproject venvs for Python. If we recommend contributors create a Python project in PyCharm or IntelliJ to work with a single integration, this won't be a problem. It looks like this will be fixed soon in IntelliJ (hopefully):
- JetBrains/intellij-community#1419
- https://intellij-support.jetbrains.com/hc/en-us/community/posts/360004233639-Detect-venv-in-the-project-folder
Our best shot
For developing Python in IntelliJ, I think the best way is to create a new project from existing sources that doesn't use Gradle at all. This basically gives up on syntax highlighting in the Gradle view of IntelliJ. We'd have two projects locally, one for Python development and one for Java/Gradle development. You can still run the Python gradle commands from the IDE if yo want. Then, we can remove the symlinked dependencies in this PR.
The default way the Python projects are created without Gradle are as separate subprojects, each using their own correct venv and correct syntax highlighting. This means there's no manual set up at all in the new project window.
After more than a day of investigation, I think this is our best bet. I'm leaving the symlinks now if you want to play around with it in the Gradle IntelliJ project, but I'll remove it if we decide to go the direction of separate project windows in IntelliJ. Since there is no way to get automatic setup in the Gradle/IntelliJ view, I think this is the best option.
Reading order:
tools/gradle/commons/integrations/python.gradle
tools/python/*
build.gradle
files