GV

GeezerVison

The World Wide Web, restored to 800 pixels of glory.

Home|About|Gaming cabinet|Archives|How it works|Starter sites

On this site

  • Sign in
  • ActionsCI/CD and automation
  • CodespacesCloud development environments
  • All Features
  • Documentation
  • Training
  • Certifications
  • Build with Copilot
  • Customize Copilot
  • Integrations
  • Trending repos
  • GitHub Sponsors
  • Developer articles
  • Enterprises

This view

  • Original page
  • New address

GeezerVison is showing github.com — gh-95132: Correctly relay *args and **kwds from sqlite3.connect to factory by erlend-aasland · Pull Request #95146 · python/cpython

Conversation

@erlend-aasland

@erlend-aasland erlend-aasland commented Jul 22, 2022 •
edited
Loading

Uh oh!

There was an error while loading. Please reload this page.

Copy link
Copy Markdown
Contributor

This PR partially reverts gh-24421 (PR) and fixes the remaining concerns
given in gh-93044 (issue).

Fixes gh-95132

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

All reactions
erlend-aasland added 2 commits July 22, 2022 19:56
@erlend-aasland
Add test
2c9e96f
@erlend-aasland
[alt 1] pythongh-95132: partially revert 185ecdc
479959f
@bedevere-bot bedevere-bot added the awaiting core review label Jul 22, 2022
@erlend-aasland
Add NEWS
507e5ca
@erlend-aasland erlend-aasland mentioned this pull request Jul 22, 2022
Python3.11: sqlite3.connect passes more arguments to factory() #95132
Closed
erlend-aasland added 3 commits July 22, 2022 21:23
@erlend-aasland
Improve test
95296cb
@erlend-aasland
[alt 2] pythongh-95132: use vectorcall
b8ef03d
@erlend-aasland
Add notes re docstrings
b144689
@erlend-aasland
erlend-aasland marked this pull request as ready for review July 22, 2022 20:13
@erlend-aasland
erlend-aasland requested a review from berkerpeksag as a code owner July 22, 2022 20:13
@erlend-aasland
erlend-aasland requested a review from serhiy-storchaka July 22, 2022 20:13
@erlend-aasland erlend-aasland added the 🔨 test-with-buildbots Test PR w/ buildbots; report in status section label Jul 22, 2022
@bedevere-bot

bedevere-bot commented Jul 22, 2022

Copy link
Copy Markdown

🤖 New build scheduled with the buildbot fleet by @erlend-aasland for commit b144689 🤖

If you want to schedule another build, you need to add the ":hammer: test-with-buildbots" label again.

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

@bedevere-bot bedevere-bot removed the 🔨 test-with-buildbots Test PR w/ buildbots; report in status section label Jul 22, 2022
@erlend-aasland

erlend-aasland commented Jul 22, 2022

Copy link
Copy Markdown
Contributor Author

Triggering the bots to be 100% sure we don't break 3.11 again.

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

@erlend-aasland
Further improve tests
bef0572
@erlend-aasland

erlend-aasland commented Jul 22, 2022

Copy link
Copy Markdown
Contributor Author

I've queued a commit that further improves the factory tests. I'll wait until the bots are done before pushing it, though 🤖

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

@erlend-aasland erlend-aasland added the needs backport to 3.11 only security fixes label Jul 22, 2022
@erlend-aasland

erlend-aasland commented Jul 23, 2022 •
edited
Loading

Uh oh!

There was an error while loading. Please reload this page.

Copy link
Copy Markdown
Contributor Author

I've queued a commit that further improves the factory tests. I'll wait until the bots are done before pushing it, though 🤖

Bot run: 87 successful and 1 skipped checks 💯

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

serhiy-storchaka
serhiy-storchaka reviewed Jul 23, 2022
View reviewed changes
Comment thread Modules/_sqlite/module.c Resolved

Uh oh!

There was an error while loading. Please reload this page.

serhiy-storchaka
serhiy-storchaka approved these changes Jul 23, 2022
View reviewed changes

@serhiy-storchaka serhiy-storchaka left a comment

Copy link
Copy Markdown
Member

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, but <unrepresentable> makes the exact signature almost useless. And there is no need to use it, because the default value can be represented.

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

erlend-aasland reacted with thumbs up emoji
All reactions
  • 👍 1 reaction
@bedevere-bot bedevere-bot added awaiting merge and removed awaiting core review labels Jul 23, 2022
@erlend-aasland

erlend-aasland commented Jul 23, 2022

Copy link
Copy Markdown
Contributor Author

LGTM, but <unrepresentable> makes the exact signature almost useless. And there is no need to use it, because the default value can be represented.

Thanks for reviewing! Yeah, that's a C&P artefact from the clinic generated docstring. I'll fix it. Thanks again!

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

Erlend Egeberg Aasland
Fix isolation_level default value
cf5da8e
@erlend-aasland

erlend-aasland commented Jul 23, 2022 •
edited
Loading

Uh oh!

There was an error while loading. Please reload this page.

Copy link
Copy Markdown
Contributor Author

This PR also addresses the remaining concerns in #93044:

  1. Keyword arguments are passed as positional arguments to factory().
  2. If an argument is not passed to sqlite3.connect(), its default value is passed to factory().
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

@serhiy-storchaka

serhiy-storchaka commented Jul 23, 2022

Copy link
Copy Markdown
Member

Both the second and the third concerns.

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

serhiy-storchaka
serhiy-storchaka reviewed Jul 23, 2022
View reviewed changes
Comment thread Modules/_sqlite/module.c Resolved

Uh oh!

There was an error while loading. Please reload this page.

Erlend Egeberg Aasland @serhiy-storchaka
Update Modules/_sqlite/module.c
ca41429
Co-authored-by: Serhiy Storchaka <[email protected]>
@erlend-aasland

erlend-aasland commented Jul 23, 2022

Copy link
Copy Markdown
Contributor Author

Both the second and the third concerns.

Yes, I've already updated my post. Also thanks for the last got'cha.

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

@erlend-aasland
erlend-aasland merged commit a3d4d15 into python:main Jul 23, 2022
@miss-islington

miss-islington commented Jul 23, 2022

Copy link
Copy Markdown
Contributor

Thanks @erlend-aasland for the PR 🌮🎉.. I'm working now to backport this PR to: 3.11.
🐍🍒⛏🤖

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

@bedevere-bot bedevere-bot removed the awaiting merge label Jul 23, 2022
@erlend-aasland
erlend-aasland deleted the sqlite-ac-connect branch July 23, 2022 07:51
@miss-islington

miss-islington commented Jul 23, 2022

Copy link
Copy Markdown
Contributor

Sorry, @erlend-aasland, I could not cleanly backport this to 3.11 due to a conflict.
Please backport using cherry_picker on command line.
cherry_picker a3d4d15f53777662ce0957500e5a538ce7161f5e 3.11

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

@miss-islington miss-islington assigned erlend-aasland Jul 23, 2022
erlend-aasland pushed a commit to erlend-aasland/cpython that referenced this pull request Jul 23, 2022
Erlend Egeberg Aasland @erlend-aasland
[3.11] pythongh-95132: Correctly relay *args and **kwds from sqlite3.…
dbee099
…connect to factory (pythonGH-95146)

This PR partially reverts pythongh-24421 (PR) and fixes the remaining concerns
given in pythongh-93044 (issue):

- keyword arguments are passed as positional arguments to factory()
- if an argument is not passed to sqlite3.connect(), its default value
  is passed to factory()

Co-authored-by: Serhiy Storchaka <[email protected]>.
(cherry picked from commit a3d4d15)

Co-authored-by: Erlend Egeberg Aasland <[email protected]>
@bedevere-bot bedevere-bot removed the needs backport to 3.11 only security fixes label Jul 23, 2022
@bedevere-bot

bedevere-bot commented Jul 23, 2022

Copy link
Copy Markdown

GH-95158 is a backport of this pull request to the 3.11 branch.

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

@erlend-aasland erlend-aasland mentioned this pull request Jul 23, 2022
Unintentional changes in sqlite3.connect() #93044
Closed
erlend-aasland pushed a commit that referenced this pull request Jul 23, 2022
Erlend Egeberg Aasland
[3.11] gh-95132: Correctly relay *args and **kwds from sqlite3.connec…
5d3f2fb
…t to factory (GH-95146) (#95158)

This PR partially reverts gh-24421 (PR) and fixes the remaining concerns
given in gh-93044 (issue):

- keyword arguments are passed as positional arguments to factory()
- if an argument is not passed to sqlite3.connect(), its default value
  is passed to factory()

Co-authored-by: Serhiy Storchaka <[email protected]>.
(cherry picked from commit a3d4d15)

Co-authored-by: Erlend Egeberg Aasland <[email protected]>
@erlend-aasland erlend-aasland mentioned this pull request Aug 8, 2022
gh-83638: Add sqlite3.Connection.autocommit for PEP 249 compliant behaviour #93823
Merged
2 tasks done
@AlexWaygood AlexWaygood mentioned this pull request Nov 2, 2022
Regression in help(sqlite3) between Python 3.10.8 and Python 3.11.0 #99003
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@serhiy-storchaka serhiy-storchaka serhiy-storchaka approved these changes
@berkerpeksag berkerpeksag Awaiting requested review from berkerpeksag

Uh oh!

There was an error while loading. Please reload this page.

Assignees

@erlend-aasland erlend-aasland

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Python3.11: sqlite3.connect passes more arguments to factory()

Uh oh!

There was an error while loading. Please reload this page.

4 participants

@erlend-aasland @bedevere-bot @serhiy-storchaka @miss-islington
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
You are visitor 01402520 — GeezerVison does not store the pages you look up.
Best viewed with Netscape Navigator 4.7 or Internet Explorer 6 at 800×600. No cookies required.