Re: [Django] #19274: Separate DB connection creation and session state initialization

[Django]

#19274: Separate DB connection creation and session state initialization
-+-
 Reporter:  akaariai |Owner:  nobody
 Type:   |   Status:  closed
  Cleanup/optimization   |  Version:  master
Component:  Database layer   |   Resolution:  fixed
  (models, ORM)  | Triage Stage:  Ready for
 Severity:  Normal   |  checkin
 Keywords:   |  Needs documentation:  0
Has patch:  1|  Patch needs improvement:  0
  Needs tests:  0|UI/UX:  0
Easy pickings:  0|
-+-
Changes (by Anssi Kääriäinen ):

 * status:  new => closed
 * resolution:   => fixed


Comment:

 In [changeset:"1893467784deb6cd8a493997e8bac933cc2e4af9"]:
 {{{
 #!CommitTicketReference repository=""
 revision="1893467784deb6cd8a493997e8bac933cc2e4af9"
 Fixed #19274 -- Made db connection creation overridable in subclasses

 Connection creation was done in db backend ._cursor() call. This
 included taking a new connection if needed, initializing the session
 state for the new connection and finally creating the connection.

 To allow easier modifying of these steps in subclasses (for example to
 support connection pools) the _cursor() now calls get_new_connection()
 and init_connection_state() if there isn't an existing connection. This
 was done for all non-gis core backends. In addition the parameters used
 for taking a connection are now created by get_connection_params().

 We should also do the same for gis backends and encourage 3rd party
 backends to use the same pattern. The pattern is not enforced in code,
 and as the backends are private API this will not be required by
 documentation either.
 }}}

-- 
Ticket URL: 
Django 
The Web framework for perfectionists with deadlines.

-- 
You received this message because you are subscribed to the Google Groups 
"Django updates" group.
To post to this group, send email to django-updates@googlegroups.com.
To unsubscribe from this group, send email to 
django-updates+unsubscr...@googlegroups.com.
For more options, visit https://groups.google.com/groups/opt_out.

Re: [Django] #19274: Separate DB connection creation and session state initialization

[Django]

#19274: Separate DB connection creation and session state initialization
-+-
 Reporter:  akaariai |Owner:  nobody
 Type:   |   Status:  closed
  Cleanup/optimization   |  Version:  master
Component:  Database layer   |   Resolution:  fixed
  (models, ORM)  | Triage Stage:  Ready for
 Severity:  Normal   |  checkin
 Keywords:   |  Needs documentation:  0
Has patch:  1|  Patch needs improvement:  0
  Needs tests:  0|UI/UX:  0
Easy pickings:  0|
-+-

Comment (by Anssi Kääriäinen ):

 In [changeset:"86644e065fbde410aa32e052e206d1c53a916fd3"]:
 {{{
 #!CommitTicketReference repository=""
 revision="86644e065fbde410aa32e052e206d1c53a916fd3"
 Refactored gis/spatialite connection initialization

 The connection state is now initialized in get_new_connection().
 Refs #19274.
 }}}

-- 
Ticket URL: 
Django 
The Web framework for perfectionists with deadlines.

-- 
You received this message because you are subscribed to the Google Groups 
"Django updates" group.
To post to this group, send email to django-updates@googlegroups.com.
To unsubscribe from this group, send email to 
django-updates+unsubscr...@googlegroups.com.
For more options, visit https://groups.google.com/groups/opt_out.

Re: [Django] #19274: Separate DB connection creation and session state initialization

[Django]

#19274: Separate DB connection creation and session state initialization
-+-
 Reporter:  akaariai |Owner:  nobody
 Type:   |   Status:  new
  Cleanup/optimization   |  Version:  master
Component:  Database layer   |   Resolution:
  (models, ORM)  | Triage Stage:  Ready for
 Severity:  Normal   |  checkin
 Keywords:   |  Needs documentation:  0
Has patch:  1|  Patch needs improvement:  0
  Needs tests:  0|UI/UX:  0
Easy pickings:  0|
-+-
Changes (by akaariai):

 * has_patch:  0 => 1
 * stage:  Accepted => Ready for checkin


Comment:

 I have update the patch series at
 https://github.com/akaariai/django/compare/ticket_19274

 Gis isn't yet updated, though it seems the only change needed is in
 spatialite. In addition I would like to remove some checks done against
 the ENGINE string, this isn't nice for subclassers.

 The full test suite does pass using
 https://github.com/akaariai/django_pooled/. The implementation in
 django_pooled isn't meant for any kind of production use, but it does
 prove that creating a connection pool outside core is possible.

 I am planning to push the patch series as-is. The changes are mostly
 mechanical - moving code from ._cursor() to the new methods. Still, it
 would be great if somebody had the time to double-check my changes, I am
 pretty good at making stupid little mistakes in this sort of thing...

-- 
Ticket URL: 
Django 
The Web framework for perfectionists with deadlines.

-- 
You received this message because you are subscribed to the Google Groups 
"Django updates" group.
To post to this group, send email to django-updates@googlegroups.com.
To unsubscribe from this group, send email to 
django-updates+unsubscr...@googlegroups.com.
For more options, visit https://groups.google.com/groups/opt_out.

Re: [Django] #19274: Separate DB connection creation and session state initialization

[Django]

#19274: Separate DB connection creation and session state initialization
-+-
 Reporter:  akaariai |Owner:  nobody
 Type:   |   Status:  new
  Cleanup/optimization   |  Version:  master
Component:  Database layer   |   Resolution:
  (models, ORM)  | Triage Stage:  Accepted
 Severity:  Normal   |  Needs documentation:  0
 Keywords:   |  Patch needs improvement:  0
Has patch:  0|UI/UX:  0
  Needs tests:  0|
Easy pickings:  0|
-+-

Comment (by akaariai):

 I have now a working implementation in:
 https://github.com/akaariai/django/compare/ticket_19274

 The work divides into three parts:
   - Backend conversion as mentioned in the ticket description
   - Removing the assumption that connection's self.settings_dict changes
 will be reflected in the global settings
   - Random test fixed (one Oracle specific fix, some fixes into how to
 create additional connections for testing and some fixes to accessing the
 connection.connection variable)

 After the fixes the work in https://github.com/akaariai/django_pooled/
 passes on all core backends. The pooling implementation in django_pooled
 is somewhere between ugly and hideous. What is interesting is that
 django_pooled shows that one can have generic pooling implementation
 outside core, and that it actually works.

 The separation of self.settings_dict from global settings is needed for
 the above pooling implementation (and I believe any pooling implementation
 wants that). The basic problem is that you need to tell Django to use the
 pool in ENGINE, and then you need to have the real engine somewhere
 (OPTIONS -> WRAPS in django_pooled). You need to alter the settings before
 passing to the real backend, but you can't do that to the global settings,
 otherwise the information is lost for next connection creation.

 Gis backends aren't converted yet.

-- 
Ticket URL: 
Django 
The Web framework for perfectionists with deadlines.

-- 
You received this message because you are subscribed to the Google Groups 
"Django updates" group.
To post to this group, send email to django-updates@googlegroups.com.
To unsubscribe from this group, send email to 
django-updates+unsubscr...@googlegroups.com.
For more options, visit https://groups.google.com/groups/opt_out.

Re: [Django] #19274: Separate DB connection creation and session state initialization

[Django]

#19274: Separate DB connection creation and session state initialization
-+-
 Reporter:  akaariai |Owner:  nobody
 Type:   |   Status:  new
  Cleanup/optimization   |  Version:  master
Component:  Database layer   |   Resolution:
  (models, ORM)  | Triage Stage:  Accepted
 Severity:  Normal   |  Needs documentation:  0
 Keywords:   |  Patch needs improvement:  0
Has patch:  0|UI/UX:  0
  Needs tests:  0|
Easy pickings:  0|
-+-
Changes (by Alex):

 * stage:  Unreviewed => Accepted


Comment:

 Sounds good to me.

-- 
Ticket URL: 
Django 
The Web framework for perfectionists with deadlines.

-- 
You received this message because you are subscribed to the Google Groups 
"Django updates" group.
To post to this group, send email to django-updates@googlegroups.com.
To unsubscribe from this group, send email to 
django-updates+unsubscr...@googlegroups.com.
For more options, visit https://groups.google.com/groups/opt_out.

[Django] #19274: Separate DB connection creation and session state initialization

[Django]

#19274: Separate DB connection creation and session state initialization
-+-
   Reporter:  akaariai   |  Owner:  nobody
   Type: | Status:  new
  Cleanup/optimization   |Version:  master
  Component:  Database   |   Keywords:
  layer (models, ORM)|  Has patch:  0
   Severity:  Normal |Needs tests:  0
   Triage Stage: |  Easy pickings:  0
  Unreviewed |
Needs documentation:  0  |
Patch needs improvement:  0  |
  UI/UX:  0  |
-+-
 Currently when creating a new connection we create the connection and
 initialize it in one go (in _cursor()). Even if a backend splits getting a
 new connection and initializing the session variable this is not done
 consistently between backends.

 The reason for this split is that this gives nice access point for
 external pooling implementations. A pool can wrap the backend and the
 methods "get_new_connection()" and "close()". Instead of actually creating
 new connections and closing the connection we can just get a connection
 from the pool and initialize the state of it. The real backend doesn't
 need to know anything about the pool wrapper.

 The pool wrapper implementation would be something like this:
 {{{
 def get_new_connection():
 if pooled_connections_available():
 return connection from pool
 else:
 return super().get_new_connection()

 def init_connection():
 do possible pool specific initialization
 super().init_connection()

 def close():
 put the connection back to pool
 }}}

 I think this would allow connection pools for Django but with no need to
 implement them in core. In addition this should add readability of
 ._cursor() implementations.

-- 
Ticket URL: 
Django 
The Web framework for perfectionists with deadlines.

-- 
You received this message because you are subscribed to the Google Groups 
"Django updates" group.
To post to this group, send email to django-updates@googlegroups.com.
To unsubscribe from this group, send email to 
django-updates+unsubscr...@googlegroups.com.
For more options, visit https://groups.google.com/groups/opt_out.