FazBrowse GitHub Viewer | Trending |
URL:
| Home
Tools: [Download Repo ZIP]   [Original HTTPS Page]

feat: support SQLAlchemy 1.4 by IlyaFaer · Pull Request #191 · googleapis/python-spanner-sqlalchemy · GitHub

This repository was archived by the owner on May 14, 2026. It is now read-only.
/ python-spanner-sqlalchemy Public archive

feat: support SQLAlchemy 1.4 - #191

Merged
vi3k6i5 merged 29 commits into
mainfrom
sqlalchemy14
Jan 27, 2022
ConversationCommits (29)ChecksFiles changed
Merged

feat: support SQLAlchemy 1.4#191
vi3k6i5 merged 29 commits into
mainfrom
sqlalchemy14

Conversation

IlyaFaer commented Jan 13, 2022 •
edited
Loading

Copy link
Copy Markdown

The first step in terms of supporting SQLAlchemy 1.4 is to prepare tests suite.

This PR splits the original test suite into two suites: 1.3 and 1.4. Though there were not much of a functionality changes between the versions, the tests suite changed significantly, thus, we need to support separate suites for the 1.3 and 1.4 versions for now.

IlyaFaer added the do not merge Indicates a pull request not ready for merge, due to either quality or timing. label Jan 13, 2022
product-auto-label Bot added the api: spanner Issues related to the googleapis/python-spanner-sqlalchemy API. label Jan 13, 2022
IlyaFaer mentioned this pull request Jan 18, 2022
Closed
vi3k6i5 linked an issue Jan 18, 2022 that may be closed by this pull request
bug: compliance_test is failing #195
Closed
IlyaFaer changed the title Sqlalchemy14 feat: support SQLAlchemy 1.4 Jan 19, 2022
IlyaFaer marked this pull request as ready for review January 19, 2022 08:52
IlyaFaer requested a review from a team January 19, 2022 08:53
IlyaFaer commented Jan 19, 2022
View reviewed changes
Comment thread .github/workflows/test_suite.yml
IlyaFaer commented Jan 19, 2022
View reviewed changes
Comment thread create_test_config.py

IlyaFaer commented Jan 19, 2022

Copy link
Copy Markdown
Author

@vi3k6i5, this better be processed in high priority, as it includes complex changes, which can affect (or be affected) by other changes.

It also requires some kokoro changes, as now there is no compliance_test session, but compliance_test_13 and compliance_test_14.

vi3k6i5 reviewed Jan 24, 2022
View reviewed changes
Comment thread test/test_suite_14.py Outdated
vi3k6i5 reviewed Jan 24, 2022
View reviewed changes
Comment thread test/test_suite_14.py Outdated
vi3k6i5 reviewed Jan 24, 2022
View reviewed changes
Comment thread test/test_suite_14.py Outdated
IlyaFaer removed the do not merge Indicates a pull request not ready for merge, due to either quality or timing. label Jan 24, 2022
IlyaFaer added the kokoro:force-run Add this label to force Kokoro to re-run the tests. label Jan 25, 2022
yoshi-kokoro removed the kokoro:force-run Add this label to force Kokoro to re-run the tests. label Jan 25, 2022
IlyaFaer added the kokoro:force-run Add this label to force Kokoro to re-run the tests. label Jan 25, 2022
yoshi-kokoro removed the kokoro:force-run Add this label to force Kokoro to re-run the tests. label Jan 25, 2022

IlyaFaer commented Jan 25, 2022

Copy link
Copy Markdown
Author

@vi3k6i5, all the checks passed except the old and unactual one (it should be dropped in the repo setting, I guess).
Can be merged now.

vi3k6i5 approved these changes Jan 25, 2022
View reviewed changes

vi3k6i5 left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

LGTM

IlyaFaer commented Jan 26, 2022

Copy link
Copy Markdown
Author

@vi3k6i5, please, merge it
I don't have admin access in the repo, so I can only merge it if all the checks passed. But we have this never executed compliance_tests, so I'm a bit helpless.

vi3k6i5 closed this Jan 27, 2022
vi3k6i5 reopened this Jan 27, 2022
vi3k6i5 reviewed Jan 27, 2022
View reviewed changes
Comment thread .kokoro/presubmit/compliance.cfg
env_vars: {
key: "NOX_SESSION"
value: "compliance_test"
value: "compliance_test_13"

vi3k6i5 Jan 27, 2022

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

@IlyaFaer : What about compliance_test_14 ? we won't enable it by default ?

vi3k6i5 commented Jan 27, 2022

Copy link
Copy Markdown
Contributor

Disabled Compliance test to merge the change as we have renamed it to compliance_test_13 and compliance_test_14. will enable it once this PR is merged.

vi3k6i5 merged commit 029b181 into main Jan 27, 2022
vi3k6i5 deleted the sqlalchemy14 branch January 27, 2022 08:10
release-please Bot mentioned this pull request Jan 27, 2022
Merged
This was referenced Jan 27, 2022
Closed
Closed

vi3k6i5 commented Jan 28, 2022

Copy link
Copy Markdown
Contributor

compliance_test_13 and compliance_test_14 are now enabled.

This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters. Learn more about bidirectional Unicode characters
Show hidden characters
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Reviewers

1 more reviewer
vi3k6i5 vi3k6i5 approved these changes
Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

api: spanner Issues related to the googleapis/python-spanner-sqlalchemy API.

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

bug: compliance_test is failing

3 participants

Back | FazBrowse Home | New Git URL