- Notifications
You must be signed in to change notification settings - Fork126
[Cleanup] Set up poetry, lockfile, mypy, and black#1
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 ourterms of service andprivacy statement. We’ll occasionally send you account related emails.
Already on GitHub?Sign in to your account
Uh oh!
There was an error while loading.Please reload this page.
Conversation
src/databricks/sql/thrift_backend.py:356: error: Not all arguments converted during string formatting
arikfr commentedMay 31, 2022
I would also revise the folder structure to be more idiomatic (doesn't have to be in this PR). |
susodapop commentedMay 31, 2022
Thanks@arikfr that's coming next :) |
| description = "Atomic file writes." | ||
| category = "dev" | ||
| optional = false | ||
| python-versions = ">=2.7, !=3.0.*, !=3.1.*, !=3.2.*, !=3.3.*" |
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.
">=2.7
are we supporting python 2?
I think pysql only support python 3. isn't that right? why do we need python 2 version of the dependency here?
| description = "Classes Without Boilerplate" | ||
| category = "dev" | ||
| optional = false | ||
| python-versions = ">=2.7, !=3.0.*, !=3.1.*, !=3.2.*, !=3.3.*, !=3.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.
ditto.
susodapop commentedMay 31, 2022 via email• edited
Loading Uh oh!
There was an error while loading.Please reload this page.
edited
Uh oh!
There was an error while loading.Please reload this page.
I would disregard any specs you see in the lockfile, as this isprocedurally generated by Poetry. The only relevant specification is inpyproject.toml which says Python 3.7 or above. …On Tue, May 31, 2022 at 16:58, Moe Derakhshani ***@***.***> wrote: ***@***.**** approved this pull request. ------------------------------ In cmdexec/clients/python/poetry.lock <#1 (comment)> : > @@ -0,0 +1,575 @@ +[[package]] +name = "atomicwrites" +version = "1.4.0" +description = "Atomic file writes." +category = "dev" +optional = false +python-versions = ">=2.7, !=3.0.*, !=3.1.*, !=3.2.*, !=3.3.*" + +[[package]] +name = "attrs" +version = "21.4.0" +description = "Classes Without Boilerplate" +category = "dev" +optional = false +python-versions = ">=2.7, !=3.0.*, !=3.1.*, !=3.2.*, !=3.3.*, !=3.4.*" ditto. — Reply to this email directly, view it on GitHub <#1 (review)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AECG7B5POD5OP5ATFA45SE3VM2DQRANCNFSM5XN64NFA> . You are receiving this because you authored the thread.Message ID: ***@***.***> |
moderakh left a comment
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.
Thanks. LGTM.
One question, not related to this PR. The name of the repo isdatabricks-sql-connector I wonder as in future we are going to have SDK for other languages (go, nodejs) how we should name the repos?
maybe${lang}-databricks-sql-connector ?
susodapop commentedMay 31, 2022
That works for me. Can I make that change right away? |
* Updated pyproject.toml using interactive `poetry init`* Specify dependencies to match those in setup.cfg* Revise dependency specifications and build a fresh lockfile.* Move .gitignore to base* Update .gitignore* Specify readme.md location (needed for Pypi)* Add mypy specification. Ignore generated files.* Fix failing mypy checks* Add black dependency* Black the codebase
Signed-off-by: Jayant Singh <jayant.singh@databricks.com>
* Updated pyproject.toml using interactive `poetry init`* Specify dependencies to match those in setup.cfg* Revise dependency specifications and build a fresh lockfile.* Move .gitignore to base* Update .gitignore* Specify readme.md location (needed for Pypi)* Add mypy specification. Ignore generated files.* Fix failing mypy checks* Add black dependency* Black the codebaseSigned-off-by: Sai Shree Pradhan <saishree.pradhan@databricks.com>
Uh oh!
There was an error while loading.Please reload this page.
Description
This PR handles a few cleanup tasks. None of these changes should affect the behaviour of the connector / package. These are administrative / build changes that will streamline build / publish in the future.
setup.cfgwithpyproject.tomlv2.0.2mypyintopyproject.tomlso type checks can be run withpoetry run mypy. Generated files from thrift are not checked.blackintopyproject.tomlso code formatting can be checked automaticallymaintainersmetadata to a Databricks distro (databricks-sql-connector-maintainers@databricks.com)These changes make way for subsequently automating build / publish using Github actions, which will be borrowed from thehttps://github.com/databricks/databricks-sql-cli repository.
How is this tested?
Manually for now:
poetry installpoetry run mypy srcpoetry run black src --checkpoetry build