Skip to content
This repository was archived by the owner on May 14, 2026. It is now read-only.

feat: support SQLAlchemy 1.4 - #191

Merged
vi3k6i5 merged 29 commits into
mainfrom
sqlalchemy14
Jan 27, 2022
Merged

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

Conversation

@IlyaFaer

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

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
vi3k6i5 reviewed Jan 24, 2022
View reviewed changes
Comment thread test/test_suite_14.py Outdated
vi3k6i5
vi3k6i5 reviewed Jan 24, 2022
View reviewed changes
Comment thread test/test_suite_14.py Outdated
vi3k6i5
vi3k6i5 reviewed Jan 24, 2022
View reviewed changes
Comment thread test/test_suite_14.py Outdated
@IlyaFaer 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 IlyaFaer added the kokoro:force-run Add this label to force Kokoro to re-run the tests. label Jan 25, 2022
@yoshi-kokoro yoshi-kokoro removed the kokoro:force-run Add this label to force Kokoro to re-run the tests. label Jan 25, 2022
@IlyaFaer IlyaFaer added the kokoro:force-run Add this label to force Kokoro to re-run the tests. label Jan 25, 2022
@yoshi-kokoro yoshi-kokoro removed the kokoro:force-run Add this label to force Kokoro to re-run the tests. label Jan 25, 2022
@IlyaFaer

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
vi3k6i5 approved these changes Jan 25, 2022
View reviewed changes

@vi3k6i5 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.

LGTM

@IlyaFaer

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 vi3k6i5 closed this Jan 27, 2022
@vi3k6i5 vi3k6i5 reopened this Jan 27, 2022
vi3k6i5
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 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.

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

@vi3k6i5

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

vi3k6i5 commented Jan 28, 2022

Copy link
Copy Markdown
Contributor

compliance_test_13 and compliance_test_14 are now enabled.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Reviewers

1 more reviewer
@vi3k6i5 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

@IlyaFaer @vi3k6i5 @yoshi-kokoro