[Django] #23546: callproc **kwargs or *args parameter
Django
----------------------------------------------+--------------------
Reporter: fatal10110 | Owner: nobody
Type: New feature | Status: new
Component: Database layer (models, ORM) | Version: 1.7
Severity: Normal | Keywords:
Triage Stage: Unreviewed | Has patch: 0
Easy pickings: 1 | UI/UX: 0
----------------------------------------------+--------------------
I think it will be useful to add kwargs or args parameters to callproc
method for those backends that support more parameters for callproc method
(e.g cx_oracle)
I can not use named parameters for my procedures with django db module.
=\\
--
Ticket URL: <https://code.djangoproject.com/ticket/23546>
Django <https://code.djangoproject.com/>
The Web framework for perfectionists with deadlines.
Django
Reporter: fatal10110 | Owner: nobody
Type: New feature | Status: new
(models, ORM) | Resolution:
Severity: Normal | Triage Stage:
Keywords: oracle | Unreviewed
Has patch: 0 | Needs documentation: 1
Needs tests: 1 | Patch needs improvement: 0
Easy pickings: 1 | UI/UX: 0
Changes (by shaib):
* keywords: => oracle
* needs_better_patch: => 0
* needs_tests: => 1
* needs_docs: => 1
--
Ticket URL: <https://code.djangoproject.com/ticket/23546#comment:1>
Django
Reporter: fatal10110 | Owner: nobody
Type: New feature | Status: new
(models, ORM) | Resolution:
Keywords: oracle | Needs documentation: 0
Has patch: 0 | Patch needs improvement: 0
Needs tests: 1 | UI/UX: 0
Easy pickings: 1 |
-------------------------------------+-------------------------------------
Changes (by aaugustin):
* needs_docs: 1 => 0
* stage: Unreviewed => Accepted
Comment:
[http://legacy.python.org/dev/peps/pep-0249/#cursor-methods PEP 249] says
that callproc only accept positional arguments.
That said, since we're just forwarding arguments to the underlying
library, this small deviation seems acceptable, with a comment explaining
it.
--
Ticket URL: <https://code.djangoproject.com/ticket/23546#comment:2>
Django
Reporter: fatal10110 | Owner:
Type: New feature | averybigant
Component: Database layer | Status: assigned
(models, ORM) | Version: 1.7
Severity: Normal | Resolution:
Keywords: oracle | Triage Stage: Accepted
Has patch: 0 | Needs documentation: 0
Needs tests: 1 | Patch needs improvement: 0
Easy pickings: 1 | UI/UX: 0
Changes (by averybigant):
* owner: nobody => averybigant
* status: new => assigned
--
Ticket URL: <https://code.djangoproject.com/ticket/23546#comment:3>
Django
Reporter: fatal10110 | Owner:
Type: New feature | averybigant
Component: Database layer | Status: assigned
Severity: Normal | Resolution:
Keywords: oracle | Triage Stage: Accepted
Needs tests: 1 | Patch needs improvement: 0
Easy pickings: 1 | UI/UX: 0
Changes (by averybigant):
* has_patch: 0 => 1
Comment:
pull request created
https://github.com/django/django/pull/3342
already tested on ubuntu 12.04 server(amd64) with oracle XE 11g
--
Ticket URL: <https://code.djangoproject.com/ticket/23546#comment:4>
Django
Reporter: fatal10110 | Owner:
Type: New feature | averybigant
Component: Database layer | Status: assigned
Severity: Normal | Resolution:
Keywords: oracle | Triage Stage: Accepted
Has patch: 1 | Needs documentation: 0
Easy pickings: 1 | UI/UX: 0
Changes (by averybigant):
* needs_tests: 1 => 0
--
Ticket URL: <https://code.djangoproject.com/ticket/23546#comment:5>
Django
Reporter: fatal10110 | Owner:
Type: New feature | averybigant
Component: Database layer | Status: assigned
Severity: Normal | Resolution:
Keywords: oracle | Triage Stage: Accepted
Has patch: 1 | Needs documentation: 0
Easy pickings: 1 | UI/UX: 0
Comment (by shaib):
Thanks for the patch. I'll take a look at it tonight (UTC+3)
--
Ticket URL: <https://code.djangoproject.com/ticket/23546#comment:6>
Django
Reporter: fatal10110 | Owner:
Type: New feature | averybigant
Component: Database layer | Status: assigned
Severity: Normal | Resolution:
Keywords: oracle | Triage Stage: Accepted
Needs tests: 0 | Patch needs improvement: 1
Easy pickings: 1 | UI/UX: 0
Changes (by shaib):
* needs_better_patch: 0 => 1
* needs_docs: 0 => 1
Comment:
Hi,
I made some notes on the PR. Additionally, documentation needed. I
suggested making the feature more general, less Oracle-specific, but even
if this path is not taken, the new behavior needs to be documented.
Currently, callproc is not documented at all, which is a shame, but
forgivable considering that the function is as defined in PEP 249;
deviating from the PEP justifies adding explicit documentation, probably
in `docs/topics/db/sql.txt`; whether or not that is done, the addition
should be mentioned in the release notes (`docs/releases/1.8.txt`).
--
Ticket URL: <https://code.djangoproject.com/ticket/23546#comment:7>
Django
Reporter: fatal10110 | Owner:
Type: New feature | averybigant
Component: Database layer | Status: assigned
Severity: Normal | Resolution:
Keywords: oracle | Triage Stage: Accepted
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 1 | UI/UX: 0
Changes (by averybigant):
* needs_better_patch: 1 => 0
* needs_docs: 1 => 0
Comment:
Replying to [comment:7 shaib]:
> Hi,
>
> I made some notes on the PR. Additionally, documentation needed. I
suggested making the feature more general, less Oracle-specific, but even
if this path is not taken, the new behavior needs to be documented.
Currently, callproc is not documented at all, which is a shame, but
forgivable considering that the function is as defined in PEP 249;
deviating from the PEP justifies adding explicit documentation, probably
in `docs/topics/db/sql.txt`; whether or not that is done, the addition
should be mentioned in the release notes (`docs/releases/1.8.txt`).
Thanks for your detailed comments. New commit pushed. I also replied your
comments on github.
--
Ticket URL: <https://code.djangoproject.com/ticket/23546#comment:8>
Django
Reporter: fatal10110 | Owner:
Type: New feature | averybigant
Component: Database layer | Status: assigned
Severity: Normal | Resolution:
Keywords: oracle | Triage Stage: Accepted
Has patch: 1 | Needs documentation: 0
Easy pickings: 1 | UI/UX: 0
Changes (by manfre):
* needs_better_patch: 0 => 1
Comment:
Implementation needs a DatabaseFeature (e.g.
{{{callproc_supports_kwargs}}} to explicitly enable support for kparams
for a backend. The current patch will pass along kparams to the underlying
backend as long as they are provided to callproc, which will raise a
confusing exception to the user. It should probably raise
{{{NotSupportedError}}}.
--
Ticket URL: <https://code.djangoproject.com/ticket/23546#comment:9>
Django
Reporter: fatal10110 | Owner:
Type: New feature | averybigant
Component: Database layer | Status: assigned
Severity: Normal | Resolution:
Keywords: oracle | Triage Stage: Accepted
Has patch: 1 | Needs documentation: 0
Easy pickings: 1 | UI/UX: 0
Changes (by averybigant):
* needs_better_patch: 1 => 0
Comment:
Replying to [comment:9 manfre]:
> Implementation needs a DatabaseFeature (e.g.
{{{callproc_supports_kwargs}}}) to explicitly enable support for kparams
for a backend. The current patch will pass along kparams to the underlying
backend as long as they are provided to callproc, which will raise a
confusing exception to the user. It should probably raise
{{{django.db.utils.NotSupportedError}}}.
Thanks. Made the changes you suggested.
--
Ticket URL: <https://code.djangoproject.com/ticket/23546#comment:10>
Django
Reporter: fatal10110 | Owner:
Type: New feature | averybigant
Component: Database layer | Status: assigned
Severity: Normal | Resolution:
Keywords: oracle | Triage Stage: Accepted
Has patch: 1 | Needs documentation: 0
Easy pickings: 1 | UI/UX: 0
Comment (by shaib):
I've added some Notes on the PR, and opened a related
[https://groups.google.com/d/msg/django-
developers/v3AhQhJFhGM/TScISVjLBQoJ mailing list discussion].
--
Ticket URL: <https://code.djangoproject.com/ticket/23546#comment:11>
Django
Reporter: fatal10110 | Owner:
Type: New feature | averybigant
Component: Database layer | Status: assigned
Severity: Normal | Resolution:
Keywords: oracle | Triage Stage: Accepted
Has patch: 1 | Needs documentation: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
Changes (by timgraham):
* needs_better_patch: 0 => 1
* easy: 1 => 0
--
Ticket URL: <https://code.djangoproject.com/ticket/23546#comment:12>
Django
Reporter: fatal10110 | Owner:
Type: New feature | Status: new
(models, ORM) |
Severity: Normal | Resolution:
Keywords: oracle | Triage Stage: Accepted
Has patch: 1 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 1
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
Changes (by timgraham):
* owner: averybigant =>
* status: assigned => new
--
Ticket URL: <https://code.djangoproject.com/ticket/23546#comment:13>
Django
Reporter: fatal10110 | Owner:
Type: New feature | Status: new
(models, ORM) |
Severity: Normal | Resolution:
Keywords: oracle | Triage Stage: Accepted
Has patch: 1 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 1
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
Comment (by theromulanz):
Is this patch still being considered for release?
I have the same use-case as the ticket description(cx_Oracle
keywordParameters).
--
Ticket URL: <https://code.djangoproject.com/ticket/23546#comment:14>
Django
Reporter: fatal10110 | Owner:
Type: New feature | Status: new
(models, ORM) |
Severity: Normal | Resolution:
Keywords: oracle | Triage Stage: Accepted
Has patch: 1 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 1
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
Comment (by timgraham):
If you'd like to help, please update the old pull request for the
comments.
--
Ticket URL: <https://code.djangoproject.com/ticket/23546#comment:15>
Django
Reporter: fatal10110 | Owner: felixxm
Type: New feature | Status: assigned
Component: Database layer | Version: master
(models, ORM) |
Severity: Normal | Resolution:
Keywords: oracle | Triage Stage: Accepted
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
* status: new => assigned
* needs_better_patch: 1 => 0
* has_patch: 1 => 0
* version: 1.7 => master
* owner: (none) => felixxm
--
Ticket URL: <https://code.djangoproject.com/ticket/23546#comment:16>
Django
Reporter: fatal10110 | Owner: felixxm
Type: New feature | Status: assigned
Component: Database layer | Version: master
(models, ORM) |
Severity: Normal | Resolution:
Keywords: oracle | Triage Stage: Accepted
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
Changes (by felixxm):
* has_patch: 0 => 1
Comment:
[https://github.com/django/django/pull/8895 PR]
--
Ticket URL: <https://code.djangoproject.com/ticket/23546#comment:17>
Django
-------------------------------------+-------------------------------------
Reporter: fatal10110 | Owner: felixxm
Type: New feature | Status: assigned
Component: Database layer | Version: master
(models, ORM) |
Severity: Normal | Resolution:
| checkin
Has patch: 1 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
* stage: Accepted => Ready for checkin
--
Ticket URL: <https://code.djangoproject.com/ticket/23546#comment:18>
Django
-------------------------------------+-------------------------------------
Reporter: fatal10110 | Owner: felixxm
Component: Database layer | Version: master
(models, ORM) |
Keywords: oracle | Triage Stage: Ready for
| checkin
Has patch: 1 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
* status: assigned => closed
* resolution: => fixed
Comment:
In [changeset:"489421b01562494ab506de5d30ea97d7b6b5df30" 489421b]:
{{{
#!CommitTicketReference repository=""
revision="489421b01562494ab506de5d30ea97d7b6b5df30"
Fixed #23546 -- Added kwargs support for CursorWrapper.callproc() on
Oracle.
Thanks Shai Berger, Tim Graham and Aymeric Augustin for reviews and
Renbi Yu for the initial patch.
}}}
--
Ticket URL: <https://code.djangoproject.com/ticket/23546#comment:19>