Skip to content

[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 our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 11 commits into from
May 31, 2022
Merged

Conversation

susodapop
Copy link
Contributor

@susodapop susodapop commented May 31, 2022

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.

  • Replaces setup.cfg with pyproject.toml
  • Locks the dependencies for v2.0.2
  • Wires in mypy into pyproject.toml so type checks can be run with poetry run mypy. Generated files from thrift are not checked.
  • Wires in black into pyproject.toml so code formatting can be checked automatically
  • Sets the maintainers metadata to a Databricks distro ([email protected])
  • Blacks the codebase

These changes make way for subsequently automating build / publish using Github actions, which will be borrowed from the https://github.com/databricks/databricks-sql-cli repository.

How is this tested?

Manually for now:

  1. Make sure poetry is installed on your system
  2. Check out this branch of the repository
  3. Run poetry install
  4. Run poetry run mypy src
  5. Run poetry run black src --check
  6. Run poetry build
  7. Within a fresh virtualenv, install the package generated in step 6

@arikfr
Copy link

arikfr commented May 31, 2022

I would also revise the folder structure to be more idiomatic (doesn't have to be in this PR).

@susodapop
Copy link
Contributor Author

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.*"
Copy link
Collaborator

@moderakh moderakh May 31, 2022

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.*"
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ditto.

@moderakh moderakh self-requested a review May 31, 2022 21:58
@susodapop
Copy link
Contributor Author

susodapop commented May 31, 2022 via email

Copy link
Collaborator

@moderakh moderakh left a 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 is databricks-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
Copy link
Contributor Author

That works for me. Can I make that change right away?

@susodapop susodapop merged commit bed1669 into main May 31, 2022
@susodapop susodapop deleted the init-poetry branch May 31, 2022 22:45
susodapop pushed a commit that referenced this pull request Jun 2, 2022
* 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
jayantsing-db added a commit to jayantsing-db/databricks-sql-python that referenced this pull request Apr 9, 2025
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.

3 participants