Re: [openstack-dev] [all] [oslo] Proposed database connectivity patterns

2014-10-10 Thread Ihar Hrachyshka
-BEGIN PGP SIGNED MESSAGE-
Hash: SHA512

On 09/10/14 21:29, Mike Bayer wrote:
 So so far, everyone seems really positive and psyched about the
 proposal.
 
 It looks like providing some options for how to use would be best,
 that is provide decorators and context managers.
 
 Now the thing with the context manager, it can be as I stated:
 
 with sql.reader() as session:
 
 or we can even have that accept the “context”:
 
 with sql.reader(context) as session:
 
 The latter again avoids having to use thread locals.
 
 But can I get a feel for how comfortable we are with using thread
 local storage to implement this feature?   I had anticipated people
 wouldn’t like it because it’s kind of a “global” object, even
 though it will be hidden behind this facade (of course CONF is
 global as is sys.modules, and I think it is fine). If I just
 use a tlocal, this whole thing is pretty simple.

Won't the approach conflict with eventlet consumers that for some
reason do not patch thread module, or do not patch it early enough? I
guess in that case we may end up with mixed contexts.

/Ihar
-BEGIN PGP SIGNATURE-
Version: GnuPG/MacGPG2 v2.0.22 (Darwin)

iQEcBAEBCgAGBQJUN7FeAAoJEC5aWaUY1u57sq0IANSM8gKCQZbgEY62uvyhZqzN
9gRDcFbTrH/yUNMv0tt3+e2vVEbn8VIatDWG4/OYghzuI1BerKzhP7JD5J+mV8yK
sB0fc6ybHmz14T962LhFkAxWKybPW3sJO/0GIy606ty0OEV8QAgPwaYvjW596MUa
BAp0IRAz4/MglT/M80OkT2jFdMV+a9SAFUKvnF/21KSGo/t4qcawA/+B0c3Ownle
eYt+Auk3hcgJnZAvCOwy7bcAb1b12gonk4nIwMl/Mik8IKNR5fnl1IqTiISUO2Jw
PV95/7muukSrt54lY+9u4SW9o/Zf/gaNCMmK3xfDmvMug6tk0SWzkzBZLva3wNc=
=04Wx
-END PGP SIGNATURE-

___
OpenStack-dev mailing list
OpenStack-dev@lists.openstack.org
http://lists.openstack.org/cgi-bin/mailman/listinfo/openstack-dev


Re: [openstack-dev] [all] [oslo] Proposed database connectivity patterns

2014-10-10 Thread Mike Bayer

On Oct 10, 2014, at 6:13 AM, Ihar Hrachyshka ihrac...@redhat.com wrote:

 Signed PGP part
 On 09/10/14 21:29, Mike Bayer wrote:
  So so far, everyone seems really positive and psyched about the
  proposal.
 
  It looks like providing some options for how to use would be best,
  that is provide decorators and context managers.
 
  Now the thing with the context manager, it can be as I stated:
 
  with sql.reader() as session:
 
  or we can even have that accept the “context”:
 
  with sql.reader(context) as session:
 
  The latter again avoids having to use thread locals.
 
  But can I get a feel for how comfortable we are with using thread
  local storage to implement this feature?   I had anticipated people
  wouldn’t like it because it’s kind of a “global” object, even
  though it will be hidden behind this facade (of course CONF is
  global as is sys.modules, and I think it is fine). If I just
  use a tlocal, this whole thing is pretty simple.
 
 Won't the approach conflict with eventlet consumers that for some
 reason do not patch thread module, or do not patch it early enough? I
 guess in that case we may end up with mixed contexts.

I’ve been asking a lot about “hey are people cool with thread locals?” and have 
been waiting for what the concerns are.

Since I wrote that email I’ve shifted, and I’ve been considering only:

@sql.reader
def my_api_method(context, …):
   context.session

def my_api_method(context, …):
  with sql.using_reader(context) as session:
session , context.session

because in fact, if you *want* to use a thread local context, you can, 
explicitly with the above:

GLOBAL_CONTEXT = threading.local()

def my_api_method(…):
  with sql.using_reader(GLOBAL_CONTEXT) as session:
session 

I like that one the best.  But again, Keystone folks would need to accept this 
explicitness.

The challenge on my end is not technical in any way.  It’s getting every 
project to agree on a single approach and not getting bogged down with 
idealistics (like, “let’s build a dependency injection framework!”).Because 
this “everyone does it their own way” thing is crazy and has to stop.





___
OpenStack-dev mailing list
OpenStack-dev@lists.openstack.org
http://lists.openstack.org/cgi-bin/mailman/listinfo/openstack-dev


Re: [openstack-dev] [all] [oslo] Proposed database connectivity patterns

2014-10-10 Thread Mike Bayer

On Oct 10, 2014, at 11:41 AM, Mike Bayer mba...@redhat.com wrote:

 I’ve been asking a lot about “hey are people cool with thread locals?” and 
 have been waiting for what the concerns are.
 
 Since I wrote that email I’ve shifted, and I’ve been considering only:
 
 @sql.reader
 def my_api_method(context, …):
   context.session
 
 def my_api_method(context, …):
  with sql.using_reader(context) as session:
session , context.session
 
 because in fact, if you *want* to use a thread local context, you can, 
 explicitly with the above:
 
 GLOBAL_CONTEXT = threading.local()
 
 def my_api_method(…):
  with sql.using_reader(GLOBAL_CONTEXT) as session:
session 
 
 I like that one the best.  But again, Keystone folks would need to accept 
 this explicitness.
 
 The challenge on my end is not technical in any way.  It’s getting every 
 project to agree on a single approach and not getting bogged down with 
 idealistics (like, “let’s build a dependency injection framework!”).
 Because this “everyone does it their own way” thing is crazy and has to stop.

I’ve now pushed these changes, as well as a summation of all the alternatives 
so far, to the latest release.  See https://review.openstack.org/#/c/125181/.



___
OpenStack-dev mailing list
OpenStack-dev@lists.openstack.org
http://lists.openstack.org/cgi-bin/mailman/listinfo/openstack-dev


Re: [openstack-dev] [all] [oslo] Proposed database connectivity patterns

2014-10-10 Thread Joshua Harlow
On Oct 10, 2014, at 3:13 AM, Ihar Hrachyshka ihrac...@redhat.com wrote:

 -BEGIN PGP SIGNED MESSAGE-
 Hash: SHA512
 
 On 09/10/14 21:29, Mike Bayer wrote:
 So so far, everyone seems really positive and psyched about the
 proposal.
 
 It looks like providing some options for how to use would be best,
 that is provide decorators and context managers.
 
 Now the thing with the context manager, it can be as I stated:
 
 with sql.reader() as session:
 
 or we can even have that accept the “context”:
 
 with sql.reader(context) as session:
 
 The latter again avoids having to use thread locals.
 
 But can I get a feel for how comfortable we are with using thread
 local storage to implement this feature?   I had anticipated people
 wouldn’t like it because it’s kind of a “global” object, even
 though it will be hidden behind this facade (of course CONF is
 global as is sys.modules, and I think it is fine). If I just
 use a tlocal, this whole thing is pretty simple.
 
 Won't the approach conflict with eventlet consumers that for some
 reason do not patch thread module, or do not patch it early enough? I
 guess in that case we may end up with mixed contexts.

Eck, this is not something people should be doing (not patching the thread 
module).

Example for why @ https://gist.github.com/harlowja/9c35e443dfa136a4f965 (run 
that and see your program lock up).

Once you accept/use eventlet, u shouldn't really be accepted half of it, 
otherwise there be dragons there; especially since openstack doesn't control 
what external library code does inside those libraries (and rightfully so it 
shouldn't), and if any library does anything like that gist code, the whole 
application will lock up...

 
 /Ihar
 -BEGIN PGP SIGNATURE-
 Version: GnuPG/MacGPG2 v2.0.22 (Darwin)
 
 iQEcBAEBCgAGBQJUN7FeAAoJEC5aWaUY1u57sq0IANSM8gKCQZbgEY62uvyhZqzN
 9gRDcFbTrH/yUNMv0tt3+e2vVEbn8VIatDWG4/OYghzuI1BerKzhP7JD5J+mV8yK
 sB0fc6ybHmz14T962LhFkAxWKybPW3sJO/0GIy606ty0OEV8QAgPwaYvjW596MUa
 BAp0IRAz4/MglT/M80OkT2jFdMV+a9SAFUKvnF/21KSGo/t4qcawA/+B0c3Ownle
 eYt+Auk3hcgJnZAvCOwy7bcAb1b12gonk4nIwMl/Mik8IKNR5fnl1IqTiISUO2Jw
 PV95/7muukSrt54lY+9u4SW9o/Zf/gaNCMmK3xfDmvMug6tk0SWzkzBZLva3wNc=
 =04Wx
 -END PGP SIGNATURE-
 
 ___
 OpenStack-dev mailing list
 OpenStack-dev@lists.openstack.org
 http://lists.openstack.org/cgi-bin/mailman/listinfo/openstack-dev


___
OpenStack-dev mailing list
OpenStack-dev@lists.openstack.org
http://lists.openstack.org/cgi-bin/mailman/listinfo/openstack-dev


Re: [openstack-dev] [all] [oslo] Proposed database connectivity patterns

2014-10-10 Thread Ihar Hrachyshka
-BEGIN PGP SIGNED MESSAGE-
Hash: SHA512

On 10/10/14 20:21, Joshua Harlow wrote:
 On Oct 10, 2014, at 3:13 AM, Ihar Hrachyshka ihrac...@redhat.com
 wrote:
 
 On 09/10/14 21:29, Mike Bayer wrote:
 So so far, everyone seems really positive and psyched about
 the proposal.
 
 It looks like providing some options for how to use would be
 best, that is provide decorators and context managers.
 
 Now the thing with the context manager, it can be as I
 stated:
 
 with sql.reader() as session:
 
 or we can even have that accept the “context”:
 
 with sql.reader(context) as session:
 
 The latter again avoids having to use thread locals.
 
 But can I get a feel for how comfortable we are with using
 thread local storage to implement this feature?   I had
 anticipated people wouldn’t like it because it’s kind of a
 “global” object, even though it will be hidden behind this
 facade (of course CONF is global as is sys.modules, and I
 think it is fine). If I just use a tlocal, this whole
 thing is pretty simple.
 
 Won't the approach conflict with eventlet consumers that for some 
 reason do not patch thread module, or do not patch it early enough?
 I guess in that case we may end up with mixed contexts.
 
 Eck, this is not something people should be doing (not patching
 the thread module).
 
 Example for why @
 https://gist.github.com/harlowja/9c35e443dfa136a4f965 (run that
 and see your program lock up).
 
 Once you accept/use eventlet, u shouldn't really be accepted half
 of it, otherwise there be dragons there; especially since
 openstack doesn't control what external library code does inside
 those libraries (and rightfully so it shouldn't), and if any
 library does anything like that gist code, the whole application
 will lock up...

Real life sucks. In Neutron, we have at least two files that patch
only some modules (though all of them patch threading). [Not that I
mean that those files should not patch the whole library set; I have
no defined view on the matter.]

Though I agree that consumers with some modules unpatched are not
something that should be seriosly considered, we're still left with
consumers that do not patch their libraries early enough. We had an
issue with tlocal when porting Neutron to oslo.messaging that resulted
in the following patch: https://review.openstack.org/#/c/96791/ The
issue showed up in very weird way hard to debug, so if possible, I
would try to avoid similar situations with oslo.db.

/Ihar
-BEGIN PGP SIGNATURE-
Version: GnuPG/MacGPG2 v2.0.22 (Darwin)

iQEcBAEBCgAGBQJUODkZAAoJEC5aWaUY1u57QrcIANLtMi1jRrfHnwV2xnwYGPOl
QwZWDG6i/cdnzqrwGz+0chqRxu4KWZB5gAfMR4/ralHvGGbaa1vGp9wtzsfIJTB1
RuyE7XKXUX4rU3WoAOB7F3uF1WzFpI8ourunBG4OCE0t0YT89z9mCLfOTZNNKGxH
OE89n4pnyMd3GXGtoxVINyA6pqQotYuHKG6o6LbhmTZ1UoL3P3rO+Onb1Ma2BF6Z
VO84smdMnOC3yVtv4TGwrW5iBoU3RwOqxooOg4g5Jam6D++kzCKrPT9wdY7TNe+C
A6IvMlad0RbygkhrLyWIWXaqCAg5VzzDvk4z6dywVkmoWS7V861mjiV4unbr0X8=
=5ovh
-END PGP SIGNATURE-

___
OpenStack-dev mailing list
OpenStack-dev@lists.openstack.org
http://lists.openstack.org/cgi-bin/mailman/listinfo/openstack-dev


Re: [openstack-dev] [all] [oslo] Proposed database connectivity patterns

2014-10-10 Thread Joshua Harlow
On Oct 10, 2014, at 12:52 PM, Ihar Hrachyshka ihrac...@redhat.com wrote:

 -BEGIN PGP SIGNED MESSAGE-
 Hash: SHA512
 
 On 10/10/14 20:21, Joshua Harlow wrote:
 On Oct 10, 2014, at 3:13 AM, Ihar Hrachyshka ihrac...@redhat.com
 wrote:
 
 On 09/10/14 21:29, Mike Bayer wrote:
 So so far, everyone seems really positive and psyched about
 the proposal.
 
 It looks like providing some options for how to use would be
 best, that is provide decorators and context managers.
 
 Now the thing with the context manager, it can be as I
 stated:
 
 with sql.reader() as session:
 
 or we can even have that accept the “context”:
 
 with sql.reader(context) as session:
 
 The latter again avoids having to use thread locals.
 
 But can I get a feel for how comfortable we are with using
 thread local storage to implement this feature?   I had
 anticipated people wouldn’t like it because it’s kind of a
 “global” object, even though it will be hidden behind this
 facade (of course CONF is global as is sys.modules, and I
 think it is fine). If I just use a tlocal, this whole
 thing is pretty simple.
 
 Won't the approach conflict with eventlet consumers that for some 
 reason do not patch thread module, or do not patch it early enough?
 I guess in that case we may end up with mixed contexts.
 
 Eck, this is not something people should be doing (not patching
 the thread module).
 
 Example for why @
 https://gist.github.com/harlowja/9c35e443dfa136a4f965 (run that
 and see your program lock up).
 
 Once you accept/use eventlet, u shouldn't really be accepted half
 of it, otherwise there be dragons there; especially since
 openstack doesn't control what external library code does inside
 those libraries (and rightfully so it shouldn't), and if any
 library does anything like that gist code, the whole application
 will lock up...
 
 Real life sucks. In Neutron, we have at least two files that patch
 only some modules (though all of them patch threading). [Not that I
 mean that those files should not patch the whole library set; I have
 no defined view on the matter.]
 

Yup it does, and once u accept eventlet it starts to become hard to get off 
it...

It's 'viral', and for better or worse openstack has got the virus... So that 
means everything that openstack touches/uses also needs to be compatible with 
that virus... It's to bad guido just didn't accept eventlet/greenlet into 
mainline python, that would of made our job easier (and then added a python 
option or something like `python -g` that turns on greenthreading by default 
when the interpreter is started up)...

Thats the crappy part with anything that monkey patches all the things, its not 
so easy to accept half of a monkey...

 Though I agree that consumers with some modules unpatched are not
 something that should be seriosly considered, we're still left with
 consumers that do not patch their libraries early enough. We had an
 issue with tlocal when porting Neutron to oslo.messaging that resulted
 in the following patch: https://review.openstack.org/#/c/96791/ The
 issue showed up in very weird way hard to debug, so if possible, I
 would try to avoid similar situations with oslo.db.

I'm unsure on this one, I don't think we should be making oslo.* that succumbs 
to the same eventlet virus just because openstack projects aren't correctly 
using eventlet. The same bug that was found in oslo.messaging could of been any 
third part library that wasn't imported in the right order in all honesty. So 
it seems like we should fix the projects (as in that review) instead of making 
the oslo.* libraries less future compatible... If eventlet/greenlet got merged 
into mainline python then I'd say probably say the reverse, but it didn't.

 
 /Ihar
 -BEGIN PGP SIGNATURE-
 Version: GnuPG/MacGPG2 v2.0.22 (Darwin)
 
 iQEcBAEBCgAGBQJUODkZAAoJEC5aWaUY1u57QrcIANLtMi1jRrfHnwV2xnwYGPOl
 QwZWDG6i/cdnzqrwGz+0chqRxu4KWZB5gAfMR4/ralHvGGbaa1vGp9wtzsfIJTB1
 RuyE7XKXUX4rU3WoAOB7F3uF1WzFpI8ourunBG4OCE0t0YT89z9mCLfOTZNNKGxH
 OE89n4pnyMd3GXGtoxVINyA6pqQotYuHKG6o6LbhmTZ1UoL3P3rO+Onb1Ma2BF6Z
 VO84smdMnOC3yVtv4TGwrW5iBoU3RwOqxooOg4g5Jam6D++kzCKrPT9wdY7TNe+C
 A6IvMlad0RbygkhrLyWIWXaqCAg5VzzDvk4z6dywVkmoWS7V861mjiV4unbr0X8=
 =5ovh
 -END PGP SIGNATURE-
 
 ___
 OpenStack-dev mailing list
 OpenStack-dev@lists.openstack.org
 http://lists.openstack.org/cgi-bin/mailman/listinfo/openstack-dev


___
OpenStack-dev mailing list
OpenStack-dev@lists.openstack.org
http://lists.openstack.org/cgi-bin/mailman/listinfo/openstack-dev


Re: [openstack-dev] [all] [oslo] Proposed database connectivity patterns

2014-10-09 Thread Roman Podoliaka
Hi Mike,

Great stuff! Fixing the mess with transactions and their scope is
probably one of the most important tasks for us, IMO. I look forward
for this to be implemented in oslo.db and consuming projects!

Thanks,
Roman

On Thu, Oct 9, 2014 at 12:07 AM, Mike Bayer mba...@redhat.com wrote:
 Hi all -

 I’ve drafted up my next brilliant idea for how to get Openstack projects to 
 use SQLAlchemy more effectively.   The proposal here establishes significant 
 detail on what’s wrong with the current state of things, e.g. the way I see 
 EngineFacade, get_session() and get_engine() being used, and proposes a new 
 system that provides a true facade around a managed context.   But of course, 
 it requires that you all change your code!  (a little bit).  Based on just a 
 few tiny conversations on IRC so far, seems like this might be a hard sell.  
 But please note, most projects are pretty much broken in how they use the 
 database - this proposal is just a first step to making it all non-broken, if 
 not as amazing and cool as some people wish it could be.  Unbreaking the code 
 and removing boilerplate is the first step - with sane and declarative 
 patterns in place, we can then build in more bells and whistles.

 Hoping you’re all super curious now, here it is!  Jump in:  
 https://review.openstack.org/#/c/125181/

 - mike







 ___
 OpenStack-dev mailing list
 OpenStack-dev@lists.openstack.org
 http://lists.openstack.org/cgi-bin/mailman/listinfo/openstack-dev

___
OpenStack-dev mailing list
OpenStack-dev@lists.openstack.org
http://lists.openstack.org/cgi-bin/mailman/listinfo/openstack-dev


Re: [openstack-dev] [all] [oslo] Proposed database connectivity patterns

2014-10-09 Thread Mike Bayer
So so far, everyone seems really positive and psyched about the proposal.

It looks like providing some options for how to use would be best, that is 
provide decorators and context managers.

Now the thing with the context manager, it can be as I stated:

with sql.reader() as session:

or we can even have that accept the “context”:

with sql.reader(context) as session:

The latter again avoids having to use thread locals.

But can I get a feel for how comfortable we are with using thread local storage 
to implement this feature?   I had anticipated people wouldn’t like it because 
it’s kind of a “global” object, even though it will be hidden behind this 
facade (of course CONF is global as is sys.modules, and I think it is fine).
 If I just use a tlocal, this whole thing is pretty simple.

I’m going to do another pass that attempts to unify these three syntaxes - I’m 
proposing calling the context manager “using_” so that it can be differentiated 
from the decorator (e.g. so each function doesn’t need to inspect arguments):

@sql.reader
def my_api_method(context, …):
context.session

def my_api_method(context, …):
   with sql.using_reader(context) as session:
 session , context.session

def my_api_method(…):
   with sql.using_reader() as session:
   session

all three will be fully interchangeable - meaning they will ultimately use 
thread local storage in any case.For now I think if 
sql.using_reader(context) or @sql.reader is called with different context 
identities in a single call stack, it should raise an exception - not that we 
can’t support that, but whether it means we should push new state onto the 
“stack” or not is ambiguous at the moment so we should refuse to guess.




On Oct 8, 2014, at 5:07 PM, Mike Bayer mba...@redhat.com wrote:

 Hi all -
 
 I’ve drafted up my next brilliant idea for how to get Openstack projects to 
 use SQLAlchemy more effectively.   The proposal here establishes significant 
 detail on what’s wrong with the current state of things, e.g. the way I see 
 EngineFacade, get_session() and get_engine() being used, and proposes a new 
 system that provides a true facade around a managed context.   But of course, 
 it requires that you all change your code!  (a little bit).  Based on just a 
 few tiny conversations on IRC so far, seems like this might be a hard sell.  
 But please note, most projects are pretty much broken in how they use the 
 database - this proposal is just a first step to making it all non-broken, if 
 not as amazing and cool as some people wish it could be.  Unbreaking the code 
 and removing boilerplate is the first step - with sane and declarative 
 patterns in place, we can then build in more bells and whistles.
 
 Hoping you’re all super curious now, here it is!  Jump in:  
 https://review.openstack.org/#/c/125181/
 
 - mike
 
 
 
 
 
 
 
 ___
 OpenStack-dev mailing list
 OpenStack-dev@lists.openstack.org
 http://lists.openstack.org/cgi-bin/mailman/listinfo/openstack-dev


___
OpenStack-dev mailing list
OpenStack-dev@lists.openstack.org
http://lists.openstack.org/cgi-bin/mailman/listinfo/openstack-dev