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

Jump to bottom

bpo-16379: Expose sqlite error code #1108

Closed
wants to merge 5 commits into from
Closed

bpo-16379: Expose sqlite error code #1108

wants to merge 5 commits into from

Conversation

@palaviv
Copy link
Contributor

@palaviv palaviv commented Apr 13, 2017 •
edited by bedevere-bot

This PR adds the sqlite error code and name to the exceptions raised by the sqlite3 module. Once this is merged my hope is to expose the sqlite extended error code as discussed in bpo-24139

https://bugs.python.org/issue16379
auvipy
auvipy suggested changes Apr 28, 2019
View changes
Copy link

@auvipy auvipy left a comment

plz rebase!

@matrixise matrixise changed the title bpo-16379: Expose sqlite error code bpo-16379: Expose sqlite error code May 7, 2019
@matrixise
Copy link
Member

@matrixise matrixise commented May 7, 2019

Hi @palaviv

Would you be interested to upgrade your PR to the last master?

Thank you

@matrixise matrixise requested a review from berkerpeksag May 7, 2019
@palaviv palaviv force-pushed the bpo-16379-expose-sqlite-error-code branch from 365ad2a to c236045 May 8, 2019
@palaviv
Copy link
Contributor Author

@palaviv palaviv commented May 8, 2019

Sure @matrixise. Do you think there is a chance for this to be merged?

@palaviv palaviv force-pushed the bpo-16379-expose-sqlite-error-code branch from b9dc556 to 31467e3 May 9, 2019
@dimaqq
Copy link
Contributor

@dimaqq dimaqq commented Jun 7, 2019

If I remember https://bugs.python.org/issue24139 correctly, the issue was rather about exposing the extended error codes http://www.sqlite.org/c3ref/c_abort_rollback.html

@palaviv
Copy link
Contributor Author

@palaviv palaviv commented Jun 11, 2019

@dimaqq you are correct. This is a patch for https://bugs.python.org/issue16379. There is a different patch that depends on this change to solve https://bugs.python.org/issue24139.

@nelsonjchen nelsonjchen mentioned this pull request Jan 12, 2020
Closed
@danielepusceddu
Copy link

@danielepusceddu danielepusceddu commented Apr 22, 2020

I'd really really like to have this feature...

dimaqq
dimaqq reviewed Apr 23, 2020
View changes
Modules/_sqlite/module.c
static const IntConstantPair _int_constants[] = {
{"PARSE_DECLTYPES", PARSE_DECLTYPES},
{"PARSE_COLNAMES", PARSE_COLNAMES},

{"SQLITE_OK", SQLITE_OK},
/* enumerated return values for sqlite3_set_authorizer() callback */
Copy link
Contributor

@dimaqq dimaqq Apr 23, 2020

Choose a reason for hiding this comment

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

It looks like SQLITE_OK is used in the wild (https://github.com/search?l=Python&p=4&q=SQLITE_OK&type=Code) thus it cannot be removed.

Copy link
Contributor

@erlend-aasland erlend-aasland Aug 6, 2021

Choose a reason for hiding this comment

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

SQLITE_OK is definitely in use. For example, it is used one of the valid return values in authoriser callbacks.

danielshahaf
danielshahaf reviewed May 24, 2020
View changes
Modules/_sqlite/module.c
#ifdef SQLITE_NOTADB
{"SQLITE_NOTADB", SQLITE_NOTADB},
#endif
{"SQLITE_DONE", SQLITE_DONE},
Copy link
Contributor

@danielshahaf danielshahaf May 24, 2020

Choose a reason for hiding this comment

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

Hi @palaviv, thanks for picking this up. Should SQLITE_NOTICE and SQLITE_WARNING be added to this list?

@erlend-aasland
Copy link
Contributor

@erlend-aasland erlend-aasland commented Apr 8, 2021

@palaviv: Can you rebase onto master and update your code to PEP 7 standards? Also, the #ifdefs are not needed as SQLite 3.7.15 is now a hard requirement.

@erlend-aasland
Copy link
Contributor

@erlend-aasland erlend-aasland commented Aug 6, 2021

@palaviv, are you planning on landing this PR? If not, would you mind if I reopened a PR with your changes cherry-picked onto it?

@erlend-aasland erlend-aasland mentioned this pull request Aug 16, 2021
Merged
@erlend-aasland
Copy link
Contributor

@erlend-aasland erlend-aasland commented Aug 30, 2021

Closing this, as #27786 (based off of this PR) has now been merged. Thank you for your work, @palaviv!

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

@dimaqq dimaqq

@danielshahaf danielshahaf

@erlend-aasland erlend-aasland

@auvipy auvipy

@berkerpeksag berkerpeksag

Assignees
No one assigned
Labels
awaiting core review CLA signed type-enhancement
Projects
None yet
Milestone
No milestone