Skip to content
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

Sqlalchemy experiments #152

Merged
merged 5 commits into from
Feb 16, 2023
Merged

Sqlalchemy experiments #152

merged 5 commits into from
Feb 16, 2023

Conversation

Valeria1235
Copy link
Contributor

@Valeria1235 Valeria1235 commented Feb 14, 2023

I hereby agree to the terms of the CLA available at: https://yandex.ru/legal/cla/?lang=en

Support ydb dialect for sqlalchemy. Work in progress.

Pull request type

  • Bugfix
  • Feature
  • Code style update (formatting, renaming)
  • Refactoring (no functional changes, no api changes)
  • Build related changes
  • Documentation content changes
  • Other (please describe):

Issue Number: ydb-platform/ydb-sqlalchemy#2

Copy link
Member

@rekby rekby left a comment

Choose a reason for hiding this comment

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

and need to pass tests

@@ -25,6 +25,12 @@ deps =
commands =
pytest -v -m "not tls" --docker-compose-remove-volumes --docker-compose=docker-compose.yml {posargs}

[testenv:py-cov]
Copy link
Member

Choose a reason for hiding this comment

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

What about add cov report to existed environments instead of create new env?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I think it's not good to slow down tests runtime on every run (and generate some files) without clear goal to gather coverage.

ydb/_dbapi/__init__.py Outdated Show resolved Hide resolved
return False

def commit(self):
pass
Copy link
Member

Choose a reason for hiding this comment

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

Is it ok to pass commit?
Is is possible to use raise NotImplementError()?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

no, sqlalchemy used it implicitly and there will raises on engine.connect

ydb/_dbapi/connection.py Outdated Show resolved Hide resolved
ydb/_sqlalchemy/__init__.py Outdated Show resolved Hide resolved
ydb/_sqlalchemy/__init__.py Outdated Show resolved Hide resolved
@Valeria1235 Valeria1235 force-pushed the _sqlalchemy_experiments branch 4 times, most recently from 1abb285 to 3f6bdf8 Compare February 15, 2023 16:31
Valeriya Popova and others added 4 commits February 15, 2023 19:31
Bumps [cryptography](https://github.com/pyca/cryptography) from 3.4.7 to 39.0.1.
- [Release notes](https://github.com/pyca/cryptography/releases)
- [Changelog](https://github.com/pyca/cryptography/blob/main/CHANGELOG.rst)
- [Commits](pyca/cryptography@3.4.7...39.0.1)

---
updated-dependencies:
- dependency-name: cryptography
  dependency-type: direct:production
...

Signed-off-by: dependabot[bot] <[email protected]>
Bumps [cryptography](https://github.com/pyca/cryptography) from 3.3.2 to 39.0.1.
- [Release notes](https://github.com/pyca/cryptography/releases)
- [Changelog](https://github.com/pyca/cryptography/blob/main/CHANGELOG.rst)
- [Commits](pyca/cryptography@3.3.2...39.0.1)

---
updated-dependencies:
- dependency-name: cryptography
  dependency-type: direct:production
...

Signed-off-by: dependabot[bot] <[email protected]>
@Valeria1235 Valeria1235 merged commit 66cfce0 into v3 Feb 16, 2023
@Valeria1235 Valeria1235 deleted the _sqlalchemy_experiments branch February 16, 2023 14:31
@Valeria1235 Valeria1235 self-assigned this Mar 21, 2023
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.

2 participants