*** openstack has joined #openstack-barbican | 16:40 | |
openstackgerrit | Louis Taylor proposed openstack/barbican: Use dictionary comprehensions and literals https://review.openstack.org/158069 | 16:48 |
---|---|---|
*** atiwari1 has joined #openstack-barbican | 16:54 | |
*** atiwari has quit IRC | 16:56 | |
*** woodster_ has quit IRC | 17:20 | |
*** woodster_ has joined #openstack-barbican | 17:24 | |
*** jaosorior has quit IRC | 17:41 | |
openstackgerrit | John Vrbanac proposed openstack/barbican: Fixing unable to retrieve req id bug https://review.openstack.org/158034 | 17:43 |
*** kgriffs|afk is now known as kgriffs | 17:50 | |
*** kgriffs is now known as kgriffs|afk | 18:40 | |
*** zz_dimtruck is now known as dimtruck | 19:48 | |
*** dave-mccowan has quit IRC | 20:05 | |
*** chlong has quit IRC | 20:23 | |
*** jaosorior has joined #openstack-barbican | 20:46 | |
jaosorior | jvrbanac: pong. Greetings from the airport | 20:48 |
jvrbanac | jaosorior, greetings | 20:49 |
jvrbanac | jaosorior, I added the context you wanted to the CR | 20:49 |
jaosorior | And I workflowed that cr about a minute ago :P | 20:50 |
jvrbanac | jaosorior, oh lol thx! | 20:50 |
jaosorior | Hehe | 20:50 |
jaosorior | How's stuff there? | 20:51 |
jvrbanac | jaosorior, pretty good. I'm trying to clean up some garbage here in Barbican. Make some of this code a little easier to understand | 20:54 |
jaosorior | Niiiiice | 20:55 |
jaosorior | What are you cleaning up? | 20:55 |
jvrbanac | jaosorior, right now, the app and database initialization | 20:55 |
jaosorior | Öhö, let's see | 20:56 |
jvrbanac | jaosorior, Paul and myself spent a decent amount of time on Friday playing around with the secret store plugin manager and realized that there are a couple race conditions around that stuff. I'll have a couple CR's up in a bit to resolve some of that stuff | 20:58 |
jaosorior | Aaah is it the stuff related the locks that he mentioned? | 20:58 |
jaosorior | Duuuuude, nice, let me know when you have that stuff up. Got about an hour and a half before I board | 20:59 |
jvrbanac | jaosorior, kind of. We removed the lock and all hell broke loose and brought up the race condition | 20:59 |
jvrbanac | jaosorior, I've gotta run a few errands here, so I probably won't get the CR's up until tonight or tomorrow | 21:00 |
jvrbanac | jaosorior, btw, how was the remaining time in Austin? | 21:03 |
jaosorior | Really good! | 21:04 |
jaosorior | Though I might have exceeded myself on the eating part | 21:04 |
jvrbanac | jaosorior, I thought that was part of the idea? ;) | 21:05 |
jaosorior | jvrbanac: hahaha anyway, it was brilliant | 21:07 |
jvrbanac | jaosorior, well, I'm glad you had a good time! Did you go exploring around Austin or did you hang out downtown? | 21:09 |
openstackgerrit | Merged openstack/barbican: Fixing unable to retrieve req id bug https://review.openstack.org/158034 | 21:10 |
jaosorior | Did some exploring and took a couple of tours. It was nice hanging out with the family | 21:11 |
jvrbanac | awesome! | 21:11 |
woodster_ | jaosorior: have a safe flight back | 21:22 |
jaosorior | woodster_: Thanks Mr.! | 21:22 |
woodster_ | jvrbanac: I'm curious what race conditions you guys ran into. Did performance improve at all? | 21:23 |
jvrbanac | woodster_, performance didn't increase, but the app startup will be more stable | 21:24 |
jvrbanac | woodster_, atleast it didn't seem to increase | 21:24 |
jaosorior | strange | 21:24 |
jvrbanac | woodster_, for one thing the secret store plugin manager was getting re-initialized all the time. Also, when we removed the lock, it showed that we were starting the db connection on first request and the config wasn't loading fast enough to initialize the db connect for the request causing the requests to fail. | 21:28 |
woodster_ | jvrbanac: I'm glad you guys are gut checking things then. The other plug in managers might have similar issues. | 21:32 |
jvrbanac | woodster_, I'll have a CR up in a bit that removes a couple layers of abstraction around db initialization and making sure we initialize our connection right before we start the app | 21:33 |
jvrbanac | woodster_, at some point, I would like for us to revisit our configuration setup. From the way I see it, we only need the per-module config options for plugins. Everything within core should be able to be specified in a config module. | 21:36 |
jvrbanac | woodster_, having multiple points of config option registration within the core system is horrible to maintain | 21:37 |
reaperhulk | yeah, it makes it so we have to lazy init everything >:( | 21:38 |
*** atiwari2 has joined #openstack-barbican | 21:40 | |
woodster_ | jvrbanac: do you mean the config parser init in app.py, or the per module config registrations? | 21:40 |
jvrbanac | woodster_, reaperhulk, the per-module config registration junk. I would suggest we centralize the core config options and then see what we can do about better handling plugin configuration | 21:42 |
reaperhulk | jvrbanac: do we do per module config outside of the plugins too? | 21:43 |
reaperhulk | If so, definitely should consolidate anything in barbican-core | 21:43 |
*** atiwari1 has quit IRC | 21:43 | |
jaosorior | jvrbanac: I suggest you add that topic to the weekly. Would be good to bring it up there | 21:43 |
jvrbanac | reaperhulk, yeah. | 21:43 |
jvrbanac | jaosorior, yep | 21:43 |
woodster_ | jvrbanac: well that is what other projects where doing in the past, and it is nice locating those settings near the logic that uses them. Not opposed to a central approach though | 21:44 |
jvrbanac | woodster_, I'll be blunt... the other projects are wrong :P | 21:45 |
jaosorior | jvrbanac: If this will ease the deployers' and admin's life, then it's probably a good idea | 21:46 |
jvrbanac | jaosorior, agreed. It also makes our lives easier as we don't have to chase config values around the app and worry about when they'll be available | 21:47 |
woodster_ | jvrbanac: just to be clear, are you seeing a race condition with the config option settings, on the CONF object? If so that could be an Oslo config bug | 21:47 |
reaperhulk | it's not a bug | 21:50 |
reaperhulk | it's a bad design decision | 21:50 |
woodster_ | jvrbanac: my recollection is that the app.py config.parse_args() call freezes the config options bases on the Barbican-API.conf file settings | 21:50 |
reaperhulk | You are correct woodster_ | 21:50 |
jvrbanac | woodster_, it's not a problem with Olso config. Because we define config options all over the place within Barbican, we can't load the configuration until all options from all loaded modules have been registered. This is only because we register those on a per module basis | 21:50 |
woodster_ | Sorry using | 21:50 |
woodster_ | My phone and distracted:) | 21:51 |
*** kgriffs|afk is now known as kgriffs | 21:51 | |
woodster_ | jvrbanac: I recall this was to support a CLI-based help process, so showing all the options you are really using in the app. Attempts to add after parse_args() will fail. At any rate give central a try. Will be a lot of files though. | 21:54 |
jvrbanac | woodster_, it shouldn't matter where options are registered | 21:58 |
reaperhulk | jvrbanac: I think woodster_'s statement is that the default option registry may not matter? It will load whatever is in the config when you call parse_args()? | 21:59 |
woodster_ | reaperhulk: that's correct, but it's been awhile since I dug into Oslo foo | 21:59 |
reaperhulk | if so, then moving the config options to be central isn't very useful since the ultimate issue will always be when we invoke parse_args | 22:00 |
reaperhulk | (Ideally we'd invoke that before module import so we could have module level variables with no lazy load required) | 22:00 |
jvrbanac | reaperhulk, woodster_ yeah. hmm... I'll have to play with it a bit and see comes out of that | 22:00 |
woodster_ | The config stuff did seem over engineered to me, just sayin' | 22:01 |
jaosorior | woodster_: Got time to review this one? https://review.openstack.org/#/c/157068/4/barbican/tests/api/test_resources.py | 22:08 |
jaosorior | I mean https://review.openstack.org/#/c/157068 | 22:08 |
openstackgerrit | John Vrbanac proposed openstack/barbican: Attempting to clean up some of the db session code https://review.openstack.org/158085 | 22:11 |
woodster_ | jaosorior Ozz could you also add a functional test for the new-way call too? | 22:22 |
jaosorior | I thought I had | 22:23 |
jaosorior | woodster_: I did | 22:23 |
jaosorior | https://review.openstack.org/#/c/157068/4/functionaltests/api/v1/behaviors/secret_behaviors.py | 22:23 |
jaosorior | this changes the secret behaviours | 22:23 |
jaosorior | and I did what we discussed | 22:23 |
jaosorior | that I would just add a couple of tests using the old way | 22:23 |
jaosorior | to show that it's still compatible | 22:24 |
jaosorior | but the new way is now the default | 22:24 |
jaosorior | in the tests | 22:24 |
woodster_ | jaosorior: oh so existing tests use the new way, got it | 22:25 |
openstackgerrit | Merged openstack/barbican: Use dictionary comprehensions and literals https://review.openstack.org/158069 | 22:26 |
kragniz | \o/ barbican patch #4! | 22:33 |
openstackgerrit | John Vrbanac proposed openstack/barbican: Attempting to clean up some of the db session code https://review.openstack.org/158085 | 22:36 |
jvrbanac | woodster_, ping | 22:45 |
woodster_ | kragniz: nice! Keep 'em coming please | 22:45 |
woodster_ | jvrbanac: I added comments to your CR | 22:46 |
jvrbanac | woodster_, yeah I just saw that. | 22:46 |
woodster_ | jvrbanac: is the main issue that the db stuff is lazy configured? If so I think your app.py method is fine sans the last two calls. The other original lines should work as they were though | 22:48 |
jvrbanac | woodster_, yeah the db being lazy configured is killing the first few calls the service takes. | 22:50 |
jvrbanac | woodster_, the point about the workers is good. I didn't realize we were using the same function for the transaction hook as for the worker db init | 22:52 |
jvrbanac | woodster_, btw, the sessions are still scoped on a per-thread/process basis | 22:59 |
*** chlong has joined #openstack-barbican | 22:59 | |
jvrbanac | woodster_, if you look at get_session, it's still creating a new scoped session | 22:59 |
woodster_ | jvrbanac also if the sequence isn't right objects linger in the session across transactions . That's why I'd try the cold start idea first, and then change those lifecycle methods | 23:00 |
woodster_ | jvrbanac I'll have to refresh my memory on the db steps | 23:07 |
jvrbanac | woodster_, I guess I'm not understanding how this is working. The way I saw it, We needed to setup the engine and session maker in the beginning, after that, just calling get_session() created us a scoped session for our thread and we were good to go from there. We really weren't using start at all except to lazy init the engine and session in the beginning | 23:09 |
*** kgriffs is now known as kgriffs|afk | 23:10 | |
woodster_ | jvrbanac: yeah I think you are right...looking at entire module now.... | 23:17 |
jvrbanac | woodster_, I think if we were to be very strict with the transaction concept, we would need to rewrite all the repos to follow something like that. However, I'm not sure that would solve the initial request failures as it would still lazy init the engine and sessions | 23:21 |
woodster_ | jvrbanac: I think your code is fine, though line 109 is not really needed as repos call that as needed. Line 152 releases the session resources so the next get-session call will start a new transaction. So really just change doctext and remove 109 | 23:28 |
jvrbanac | woodster_, ok. I also, do need to make sure the worker initializes correctly as well. | 23:29 |
woodster_ | jvrbanac: you can just call your setup call from the bin scripts I think, one for worker one for keystone listener | 23:31 |
jvrbanac | woodster_, Hmm... yeah I see it. I suppose we could also do it in the TaskServer constructor | 23:35 |
jvrbanac | woodster_, would someone run the TaskServer outside of barbican-worker.py? | 23:36 |
woodster_ | jvrbanac: those bin scripts boot things up (there will be multiple worker-like processes like that eventually) so if you can find a common code to put the init in that would be best | 23:41 |
jvrbanac | woodster_, ok. will do | 23:42 |
woodster_ | jvrbanac: nice thanks | 23:46 |
Generated by irclog2html.py 2.14.0 by Marius Gedminas - find it at mg.pov.lt!