Skip to content

Refactored create_connect_args to fix a few bugs. - #199

Merged
bimalkjha merged 1 commit into
ibmdb:masterfrom
bdholder:master
Aug 26, 2026
Merged

Refactored create_connect_args to fix a few bugs.#199
bimalkjha merged 1 commit into
ibmdb:masterfrom
bdholder:master

Conversation

@bdholder

Copy link
Copy Markdown
Contributor

Setting Python 3.9 as the earliest supported version sets the minimum supported SQLAlchemy version to 1.3.5. Versions 1.3.4 and earlier import time.clock, a standard library function that was deprecated in Python 3.3 and removed in 3.8.

In the interest of making minimal changes, I have only tweaked setup.py to reflect these dependencies rather than migrating to pyproject.toml. In ibm_db.py, I refactored the imports to take advantage of type annotations and remove unused imports from SQLAlchemy. The only other changes wererefactoring create_connect_args to fix the query parameter bug as well as some bugs that would be triggered by passing None values to ibm_db_dbi.connect and adding a warning about password truncation. I also added a very basic test module to validate the output of create_connect_args.

@bchoudhary6415

Copy link
Copy Markdown
Collaborator

Thanks for the PR, @bdholder,

the core fix in create_connect_args is solid. Passing all query parameters through via params |= url.query (instead of the old hardcoded whitelist) correctly resolves #198, and switching to url.translate_connect_args(...) is a clean approach. Before we can merge, could you please address the following:

  1. Please remove the new test file under test/ test/test_create_connect_args.py isn't something we want to carry in the repo. Please drop that file from the PR, the ibm_db.py fix and setup.py change are what we want to keep.

  2. DCO sign-off is failing The DCO check on this PR is failing (checks). Please sign off your commit(s) so the check passes. You can amend and force-push:

git commit --amend --signoff
git push --force-with-lease

For multiple commits, use git rebase --signoff HEAD~ (or rebase and add Signed-off-by: Name to each commit). The sign-off email must match your commit author email.

  1. Please keep the original SQLAlchemy version handling The refactor removes the if SA_VERSION_MM < (2, 0): ... else: ... (and < (0, 8)) conditional imports and now unconditionally does:
    from sqlalchemy.engine import result as _result

along with dropping the conditional processors import. This is a problem because _result.ResultProxy (used in get_result_proxy) does not exist in SQLAlchemy 2.0, and the processors import location also differs by version (the same version-conditional pattern is used in ibm_db_as400.py). We need to keep supporting both SQLAlchemy 1.x and 2.x, so please retain the original version-conditional import handling (SA_VERSION_MM < (2, 0) / < (0, 8) branching for processors, _result, and ResultProxy) rather than removing it.

Note on setup.py: the python_requires=">=3.9" addition is fine and necessary, since the new code uses 3.9+ syntax (tuple[...], dict[...], |= on dicts), just want to confirm that dropping <3.9 support is intentional.

Once the test file is removed, DCO is signed off, and the original SQLAlchemy version handling is preserved, this should be good to go.

Thanks!

primarily query parameters being discarded.

Signed-off-by: Benjamin Holder <bholder@rpagency.com>
@bdholder

Copy link
Copy Markdown
Contributor Author

I added the Python version lower bound on the basis of your statement that the Python compatibility targets are 3.9 through 3.14. While you also stated an intent to support SQLAlchemy 0.7.3 through 2.0.x, this is impossible. The latest 0.7 release, 0.7.10, imports sqlalchemy/exc.py which uses obsolete syntax that was changed in Python 3.0, so the code won't compile. Since installing a version < 0.8 isn't possible under Python 3, that code branch is dead and can be removed.

I've restored the conditional import for versions less than 2.0.

@bchoudhary6415

Copy link
Copy Markdown
Collaborator

Hi @bimalkjha ,
Please merge this PR.

Thank you!

@bchoudhary6415 bchoudhary6415 linked an issue Aug 26, 2026 that may be closed by this pull request
@bimalkjha
bimalkjha merged commit 0513233 into ibmdb:master Aug 26, 2026
3 checks passed
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.

Bug: most query parameters are silently discarded

3 participants