| *** jmlowe has quit IRC | 00:21 | |
| *** jmlowe has joined #openstack-glance | 00:23 | |
| *** whoami-rajat__ has quit IRC | 00:50 | |
| *** jv__ has joined #openstack-glance | 00:52 | |
| *** k_mouza has joined #openstack-glance | 00:52 | |
| *** k_mouza has quit IRC | 00:56 | |
| *** jv__ has quit IRC | 01:08 | |
| *** Liang__ has joined #openstack-glance | 01:19 | |
| *** baojg has joined #openstack-glance | 01:47 | |
| *** baojg has quit IRC | 01:50 | |
| *** gyee has quit IRC | 01:58 | |
| *** rcernin has quit IRC | 02:56 | |
| *** whoami-rajat__ has joined #openstack-glance | 02:57 | |
| *** rcernin has joined #openstack-glance | 03:04 | |
| *** jdillaman has quit IRC | 03:29 | |
| *** baojg has joined #openstack-glance | 03:31 | |
| openstackgerrit | Merged openstack/glance master: Disable wait_for_fork() kill aggression if expect_exit=True https://review.opendev.org/747991 | 03:34 |
|---|---|---|
| *** baojg has quit IRC | 03:35 | |
| *** rcernin has quit IRC | 03:35 | |
| *** Liang__ has quit IRC | 03:37 | |
| *** Liang__ has joined #openstack-glance | 03:38 | |
| *** rcernin has joined #openstack-glance | 03:39 | |
| *** rcernin has quit IRC | 03:54 | |
| *** ratailor has joined #openstack-glance | 03:58 | |
| *** rcernin has joined #openstack-glance | 04:08 | |
| *** rcernin has quit IRC | 04:18 | |
| *** rcernin has joined #openstack-glance | 04:19 | |
| *** evrardjp has quit IRC | 04:33 | |
| *** evrardjp has joined #openstack-glance | 04:33 | |
| *** udesale has joined #openstack-glance | 05:48 | |
| *** Liang__ has quit IRC | 06:01 | |
| *** Liang__ has joined #openstack-glance | 06:04 | |
| *** Liang__ has quit IRC | 06:57 | |
| *** belmoreira has joined #openstack-glance | 07:01 | |
| *** irclogbot_2 has quit IRC | 07:13 | |
| *** Luzi has joined #openstack-glance | 07:37 | |
| *** ralonsoh has joined #openstack-glance | 07:59 | |
| openstackgerrit | zhufl proposed openstack/glance master: [Trivial]Add missing print parameters in log messages https://review.opendev.org/748585 | 08:14 |
| *** Luzi has quit IRC | 08:40 | |
| *** Liang__ has joined #openstack-glance | 09:09 | |
| *** ralonsoh_ has joined #openstack-glance | 09:21 | |
| *** ralonsoh_ has quit IRC | 09:21 | |
| *** ratailor has quit IRC | 09:30 | |
| *** k_mouza has joined #openstack-glance | 09:39 | |
| *** Liang__ has quit IRC | 10:04 | |
| *** Liang__ has joined #openstack-glance | 10:05 | |
| *** Liang__ has quit IRC | 10:07 | |
| *** jv__ has joined #openstack-glance | 11:22 | |
| *** udesale has quit IRC | 11:39 | |
| *** udesale has joined #openstack-glance | 11:43 | |
| *** k_mouza has quit IRC | 11:43 | |
| *** udesale has quit IRC | 11:47 | |
| *** udesale has joined #openstack-glance | 11:48 | |
| openstackgerrit | Vladyslav Drok proposed openstack/glance_store master: [RBD] Parallelize writes on image creation https://review.opendev.org/748607 | 11:55 |
| openstackgerrit | Merged openstack/glance stable/ussuri: Fix import failure status reporting when all_stores_must_succeed=True https://review.opendev.org/748012 | 11:56 |
| openstackgerrit | Merged openstack/glance stable/ussuri: Fix non-deterministic copy_image_revert_lifecycle test https://review.opendev.org/748239 | 11:56 |
| openstackgerrit | Merged openstack/glance stable/ussuri: Poll for final state on test_copy_image_revert_lifecycle() https://review.opendev.org/748240 | 11:56 |
| openstackgerrit | Merged openstack/glance stable/ussuri: Add context.elevated() helper for getting admin privileges https://review.opendev.org/748013 | 11:56 |
| openstackgerrit | Merged openstack/glance stable/ussuri: Implement time-limited import locking https://review.opendev.org/748014 | 11:56 |
| *** jv__ has quit IRC | 12:38 | |
| *** rcernin has quit IRC | 12:56 | |
| *** jv__ has joined #openstack-glance | 13:09 | |
| *** k_mouza has joined #openstack-glance | 13:27 | |
| *** jv__ has quit IRC | 13:48 | |
| *** k_mouza has quit IRC | 13:49 | |
| *** k_mouza has joined #openstack-glance | 13:54 | |
| *** udesale has quit IRC | 13:54 | |
| dansmith | abhishekk: I always thought get_flow() was run synchronously with the API call, but the log I'm looking at makes it seem like that is *not* the case... | 14:00 |
| *** k_mouza has quit IRC | 14:18 | |
| *** k_mouza has joined #openstack-glance | 14:20 | |
| abhishekk | dansmith, right that's not the case | 14:46 |
| dansmith | that's unfortunate | 14:47 |
| abhishekk | why is that, I am still going through your comments on tempest patch | 14:50 |
| dansmith | well, because there's really no reliable prep work done during the API call, and because you can't even depend on the os_glance_importing_to_stores to have been initialized after the API request is done | 14:51 |
| dansmith | I get that 202 lets you disavow all responsibility for the behavior, and that it makes it easier to not commit to anything, | 14:51 |
| dansmith | but if people have a hard time writing solid client code for your API because you didn't make any guarantees, it's likely to frustrate everyone | 14:52 |
| dansmith | in a lot of cases, the people writing client code for a cloud service aren't going to be as skilled as those writing the actual service, so making the workflow as predictable and understandable as possible just generally increases the ability for people to consume your service | 14:53 |
| dansmith | (I'm using "you" in the royal sense here, not _you_ specifically) | 14:53 |
| abhishekk | (got it) | 14:54 |
| dansmith | (good :P) | 14:54 |
| abhishekk | So, I think they have task_id with them at that moment, righjt? | 14:54 |
| abhishekk | can't they use that task_id to see the status of task? | 14:55 |
| dansmith | only now that we've added the lock, and only if you're an admin AFAIK | 14:55 |
| abhishekk | I got your point though how frustrating this will be | 14:55 |
| dansmith | gmann is a pretty skilled api client author, so he's a very good data point on the best you will expect from other people doing it | 14:57 |
| dansmith | and even I thought it was synchronous from writing my own tests for it, and being familiar with the code, despite losing track of the actual task creation through the layers of the onion | 14:57 |
| abhishekk | onion it is, its not that easy as you see | 14:58 |
| dansmith | I do think that maybe making tasks open to the users that created them might be a good thing to work on for W | 14:58 |
| abhishekk | +1, I am going to add that in the discussion list | 14:59 |
| dansmith | I haven't looked at what all would be required for that, but as rosmaita suggested, returning a handle to a task-id directly from the import call without them even having to look at the task lock field would be nice | 14:59 |
| dansmith | cool, because as you saw from some of my synchrinous api tests, I had to use that to keep track of things | 14:59 |
| dansmith | *synchronous | 14:59 |
| abhishekk | Yes | 14:59 |
| gmann | yeah, i was checking it from 'store' in initial test writing and then switched to 'os_glance_importing_to_stores' by seeing it from the API response from other import call | 15:00 |
| dansmith | yeah | 15:01 |
| dansmith | 202 doesn't mean you can't do anything useful before the API call returns, but I think people too often use 202 as a "I have made no promises so I can make all the stuff after the 202 totally fragile" | 15:01 |
| gmann | cannot we return some 'unknown' or 'empty list' for this field which can be assumed as ' you need to wait for some other status to decide anything on requested operation with 202' | 15:02 |
| dansmith | well, | 15:03 |
| dansmith | the way it works on the server side is that an image doesn't even have that key until the first time it's set by an import task | 15:03 |
| dansmith | I thought that the task setup was called synchronously since we get the task_id right away, and that's what creates it, | 15:03 |
| dansmith | but apparently that can happen async and all we have is the task_id by the time we return | 15:03 |
| dansmith | if we make the API initialize those fields, it needs to do so before the task could start running so we don't race, but after we gain the lock for the image | 15:04 |
| gmann | yeah but 'not-started' or 'empty list' or some more good name can also be interpreted as 'no import started/requested' or 'completed yet' | 15:05 |
| dansmith | so, yeah, we could, but I'm not sure it's better because it'll then just be an empty list and if your code runs until the "in progress" list is empty, | 15:05 |
| dansmith | you'd exit immediately | 15:05 |
| dansmith | not really, because ^ | 15:05 |
| dansmith | the change I made this morning uses the "list not even present yet" to indicate we should keep polling | 15:05 |
| dansmith | i.e. "if the list is present *and* it's empty, then we're done" | 15:05 |
| gmann | yeah but that has to be done on client side which many clients/scripts get confused and drop the idea of monitoring this dynamically added field (same you mentioned in initial chat) | 15:07 |
| dansmith | this was my point about this being hard for the user to reason about, not knowing all the details in the backend.. they have to learn by experimentation that they need to account for all these states" when it appears from the API docs that all they need to do is wait for the store to disappear from the "in progress" list and then check "failed" or "stores" to determine success | 15:07 |
| gmann | exactly | 15:07 |
| dansmith | gmann: yeah, that's why the right fix would be to have the list set properly on API return | 15:07 |
| gmann | yeah. | 15:11 |
| abhishekk | that's also need to set once lock is acquired | 15:12 |
| dansmith | but before we start the task, | 15:13 |
| dansmith | which we can't really do, AFAIK | 15:13 |
| dansmith | because we can't set the lock until we have the task_id, | 15:13 |
| abhishekk | yes, difficult with current design | 15:14 |
| dansmith | but after we have the task_id, the task could be running, and we don't want to race | 15:14 |
| abhishekk | right | 15:14 |
| abhishekk | smcginnis, when you get some time, please look at this chain https://review.opendev.org/#/c/745566/9 | 15:14 |
| *** belmoreira has quit IRC | 15:17 | |
| *** rosmaita has quit IRC | 15:22 | |
| *** rosmaita has joined #openstack-glance | 15:25 | |
| * abhishekk dinner break | 15:35 | |
| * abhishekk back | 16:18 | |
| *** k_mouza has quit IRC | 16:19 | |
| *** k_mouza has joined #openstack-glance | 16:38 | |
| *** ralonsoh has quit IRC | 16:48 | |
| *** k_mouza has quit IRC | 16:48 | |
| *** k_mouza has joined #openstack-glance | 16:49 | |
| *** k_mouza has quit IRC | 16:53 | |
| openstackgerrit | Mohammed Naser proposed openstack/glance_store master: Drop snapshot in use log from ERROR to WARN https://review.opendev.org/748723 | 17:15 |
| mnaser | dansmith: ^ a trivial make-life-easy thing that we started catching since we started logging all errors in openstack servies | 17:16 |
| dansmith | it's about to raise, so error would generally be expected there.. does that get caught and retried somewhere? | 17:17 |
| abhishekk | no | 17:19 |
| dansmith | looks like it's going to log error anyway on L620 right? | 17:19 |
| dansmith | er, no, that's something else | 17:20 |
| openstackgerrit | Mohammed Naser proposed openstack/glance_store master: Drop snapshot in use log from ERROR to WARN https://review.opendev.org/748723 | 17:20 |
| dansmith | abhishekk: your "no" was for what, retried? | 17:22 |
| abhishekk | dansmith, yes | 17:22 |
| dansmith | abhishekk: does that raise cause a stack trace in the logs? | 17:22 |
| abhishekk | dansmith, No, its caught at glance side | 17:24 |
| dansmith | yeah I see, which logs as warning also | 17:24 |
| abhishekk | warning is logged there and 409 is raised | 17:24 |
| abhishekk | mnaser, you need to change that message from _LE to _LW as well | 17:25 |
| mnaser | dansmith: yeah it's caught by glance and handled just fine, but it would end up showing up as an error log message (where tbh it should be more of a warning) | 17:25 |
| mnaser | IMHO error should be operator actionable, this usually isn't | 17:25 |
| dansmith | yeah, I see | 17:26 |
| openstackgerrit | Mohammed Naser proposed openstack/glance_store master: Drop snapshot in use log from ERROR to WARN https://review.opendev.org/748723 | 17:26 |
| mnaser | abhishekk: ^ addressed that there | 17:26 |
| abhishekk | mnaser, ack, thank you | 17:27 |
| * abhishekk signing out for the day, have a nice weekend all o/ | 17:36 | |
| *** whoami-rajat__ has quit IRC | 18:56 | |
| *** whoami-rajat__ has joined #openstack-glance | 20:17 | |
| openstackgerrit | Merged openstack/glance_store master: Drop snapshot in use log from ERROR to WARN https://review.opendev.org/748723 | 20:40 |
| -openstackstatus- NOTICE: A zuul server ended up with read only filesystems which caused many jobs to hit retry_limit. The server has been rebooted and appears happy. Jobs can be rechecked. | 22:13 | |
| *** whoami-rajat__ has quit IRC | 22:46 | |
| *** rcernin has joined #openstack-glance | 23:10 | |
| *** rcernin has quit IRC | 23:15 | |
Generated by irclog2html.py 2.17.2 by Marius Gedminas - find it at https://mg.pov.lt/irclog2html/!