- Notifications
You must be signed in to change notification settings - Fork126
[Cleanup] Move connector files to root#2
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
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.
As we are re-structuring the files, and there is no CI running on the PR at the moment, we potentially may break something.
I wonder when you are planning to enable a CI on the repo? Is it possible to enable any sort of basic CI before doing any more structural changes? (maybe for start just a CI to run the basic unit test but not the end to end test)? (this doesn't need to be in this PR) what do you think@susodapop?
| cryptography==35.0.0 | ||
| thrift==0.13.0 | ||
| pandas==1.3.4 | ||
| future==0.18.2 |
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.
I see this file is not moved, but removed. isn't the list of dependencies, dev_requirements.txt required anymore?
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.
After we merged#1, the dev environment specification is contained inpyproject.toml so there's no need for a separatedev_requirements.txt file. If we need to add more development dependencies in the future we can do that from the command line withpoetry add <package name> --dev.
| @@ -1,31 +0,0 @@ | |||
| [metadata] | |||
| name = databricks-sql-connector | |||
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.
did another file replace this file now?
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.
Yep, as part of#1 we replacedsetup.cfg withpyproject.toml since we're usingpoetry instead ofdistutils for build / publish now.
susodapop commentedJun 2, 2022
Good points about unit testing. We can't add automated CI tests without additional commits. But I can run these tests manually. I'm working on that now. |
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.
LGTM. thanks@susodapop
unit tests to fail.
Base.py alembic debugging line removed
* Move all files from cmdexec/clients/python -> root* Remove unused config files* Move HISTORY to root* Rename HISTORY to changelog and include in package.* Add Apache 2.0 license text
* Move all files from cmdexec/clients/python -> root* Remove unused config files* Move HISTORY to root* Rename HISTORY to changelog and include in package.* Add Apache 2.0 license textSigned-off-by: Sai Shree Pradhan <saishree.pradhan@databricks.com>
Description
This PR moves all the source code into the root of the repository. The previous nesting was the result of retaining the commit history when this connector was part of another Databricks repository.
HISTORY->CHANGELOG.mdpyproject.tomlso that the CHANGELOG will be copied into the distribution packageLICENSEfile with the Apache 2.0 license text.