| opendevreview | Mridula Joshi proposed openstack/glance-specs master: Append new tags to existing tags https://review.opendev.org/c/openstack/glance-specs/+/813429 | 04:53 |
|---|---|---|
| opendevreview | Takashi Kajinami proposed openstack/glance master: Use LOG.warning instead of deprecated LOG.warn https://review.opendev.org/c/openstack/glance/+/819599 | 07:38 |
| opendevreview | Takashi Kajinami proposed openstack/glance master: Use LOG.warning instead of deprecated LOG.warn https://review.opendev.org/c/openstack/glance/+/819599 | 08:08 |
| opendevreview | Takashi Kajinami proposed openstack/glance master: Use LOG.warning instead of deprecated LOG.warn https://review.opendev.org/c/openstack/glance/+/819599 | 08:08 |
| opendevreview | mitya-eremeev-2 proposed openstack/glance-specs master: Spec Lite: Add ability to purge all rows by glance-manage https://review.opendev.org/c/openstack/glance-specs/+/819622 | 11:16 |
| opendevreview | mitya-eremeev-2 proposed openstack/glance-specs master: Spec Lite: Add ability to purge all rows by glance-manage https://review.opendev.org/c/openstack/glance-specs/+/819622 | 11:35 |
| *** tosky is now known as Guest7142 | 13:37 | |
| *** tosky_ is now known as tosky | 13:37 | |
| dansmith | whoami-rajat: sorry I missed your ping, holiday in the us | 14:38 |
| whoami-rajat | dansmith, hey, np, i wanted to discuss the functional tests of cinder we have on glance side, that seems to be a problem every time i do a change in the cinder store | 14:49 |
| whoami-rajat | dansmith, we added them in victoria for the legacy image migration testing, I was thinking do we still need them in master | 14:49 |
| whoami-rajat | dansmith, also we don't have any other store related functional tests in glance | 14:50 |
| dansmith | whoami-rajat: don't they kinda highlight when you're actually breaking something though? or are you talking about the ones that mock the cinderclient behavior? | 14:51 |
| whoami-rajat | dansmith, that's the cross project glance functional job that signifies the breaking, the breaking thing are the tests https://github.com/openstack/glance/blob/master/glance/tests/functional/v2/test_legacy_update_cinder_store.py | 14:53 |
| whoami-rajat | for instance this is the current change failing https://review.opendev.org/c/openstack/glance_store/+/818722 | 14:54 |
| whoami-rajat | and this is the fix in glance https://review.opendev.org/c/openstack/glance/+/818966 | 14:55 |
| whoami-rajat | but both are failing gate | 14:56 |
| whoami-rajat | and have dependency on each other | 14:57 |
| dansmith | yeah, but the reason for this is that this is three trees that are all very very closely dependent on each other | 14:59 |
| dansmith | which maybe isn't the best plan in general, | 14:59 |
| dansmith | but the reason it's hard to make a change to one and not the other is because they're so tightly integrated | 15:00 |
| dansmith | so dropping this test is "easy" but it also does reduce our coverage it seems, unless you're totally sure that you cover all the cases in an integrated job | 15:00 |
| dansmith | I guess that wasn't really possible for the migration, so that's why we have these yeah? | 15:00 |
| whoami-rajat | dansmith, so we've 3 cases, 1) when admin performs the migration, 2) when image non-owner performs the migration 3) create image | 15:02 |
| whoami-rajat | i think create image should be a generic functional test for all stores (we might have one already) | 15:03 |
| whoami-rajat | and we introduced these tests for single -> multiple store migration in victoria | 15:03 |
| dansmith | by "create image" you mean we confirm that when you create an image, we use the new thing from the start right? i.e no migration | 15:04 |
| whoami-rajat | yes | 15:04 |
| whoami-rajat | https://github.com/openstack/glance/blob/master/glance/tests/functional/v2/test_legacy_update_cinder_store.py#L157-L158 | 15:04 |
| whoami-rajat | but again my point is, these are too specific to the cinder store and as you said tightly coupled | 15:05 |
| dansmith | yeah, I understand | 15:06 |
| dansmith | so we have this potential as long as you can configure glance as single or multi store right? | 15:06 |
| dansmith | because they could deploy xena as single and then reconfigure as multi and we'd need this migration code yeah? | 15:07 |
| whoami-rajat | yeah, that's the only reason to keep it, that we allow single store | 15:09 |
| whoami-rajat | but didn't we do this location URL changes in other stores as well? | 15:10 |
| dansmith | I dunno | 15:10 |
| dansmith | sounds like if we get rid of single store (per the plan I think) then we could at *least* drop these then | 15:11 |
| dansmith | without a reconfigure job, I'm not sure how we could test that this actually works in an integrated job | 15:11 |
| whoami-rajat | abhishekk, can you confirm? do we have location changes for other stores or just cinder store? ^^ | 15:12 |
| dansmith | and at the same time, if we're expecting a bunch of people to convert from single->multistore then it would be really (really) bad if this was broken when they tried :) | 15:12 |
| abhishekk | cinder only | 15:13 |
| abhishekk | because cinder location was just cinder://image-id | 15:13 |
| whoami-rajat | ack | 15:14 |
| abhishekk | also dropping single store support will take more that one cycle (if we/me started to work on it now) | 15:15 |
| abhishekk | s/that/than | 15:15 |
| whoami-rajat | ack, so I'm not sure about the way forward since most of my glance store patches gets blocked by these tests | 15:16 |
| dansmith | whoami-rajat: this can go in with no depends-on right? https://review.opendev.org/c/openstack/glance/+/818966 | 15:17 |
| dansmith | if you merge the mocks first and then start using them, will that break the chain? | 15:17 |
| abhishekk | last time I remember we have done some additional mocking with comments? | 15:18 |
| whoami-rajat | the mock fails since strutils will only be available after glance changes are released | 15:18 |
| dansmith | yeah, you just have to plan the dance properly so that you do things in a proper order | 15:18 |
| dansmith | glance-store you mean? | 15:19 |
| dansmith | I agree that the fact that glance-store is a library makes this very difficult :/ | 15:19 |
| whoami-rajat | abhishekk, that was for backward compatibility but this imports a new library | 15:19 |
| whoami-rajat | dansmith, yes, glance-store, i tested the glance tests locally, it only passes when i copy the my glance store changes in the tox env directory | 15:20 |
| abhishekk | can't we use oslo_utils, does it have that method? | 15:20 |
| dansmith | yeah this should be mocking the actual oslo thing and not cinderclient's import of it | 15:21 |
| dansmith | whoami-rajat: maybe this should be mocking the entire cinderclient module and not trying to use part of it and mock other parts? | 15:22 |
| whoami-rajat | abhishekk, you mean mock oslo_utils? | 15:23 |
| abhishekk | yes | 15:23 |
| whoami-rajat | dansmith, I'm mocking the cinder store's import of it, i can try mocking the oslo_utils part | 15:24 |
| whoami-rajat | abhishekk, ack, will try that | 15:24 |
| dansmith | whoami-rajat: you're mocking cinder's import of it now, | 15:24 |
| dansmith | I'm saying do what abhishekk is saying and mock the oslo bit itself | 15:24 |
| whoami-rajat | ack, will try that | 15:24 |
| abhishekk | ack, I will revisit my devstack patches which are required to deploy multistore for swift and ceph, so that we can drop single store deployment | 15:26 |
| whoami-rajat | yep, that worked, thanks abhishekk dansmith , will update the patch | 15:31 |
| dansmith | whoami-rajat: sweet :) | 15:31 |
| abhishekk | cool | 15:31 |
| dansmith | reminder that this is my last working week of the year :/ | 15:32 |
| abhishekk | :o | 15:32 |
| opendevreview | Rajat Dhasmana proposed openstack/glance master: Fix tests for logging connection info https://review.opendev.org/c/openstack/glance/+/818966 | 15:32 |
| abhishekk | dansmith, ack, thanks for the heads up, probably need your views on couple of open specs and RBAC stuff which we need to carry till you will be back | 15:34 |
| dansmith | yeah, point me at the important things | 15:34 |
| whoami-rajat | abhishekk, dansmith also there is one small patch with dansmith 's suggestion, if you've some time to take a look https://review.opendev.org/c/openstack/glance/+/805974 | 15:34 |
| dansmith | I already have a very full list of stuff to get to this week, as you can imagine :( | 15:35 |
| abhishekk | whoami-rajat, will take a look | 15:35 |
| abhishekk | dansmith, yes, I know | 15:35 |
| whoami-rajat | ack, thanks. and thanks again for the help :) | 15:35 |
| abhishekk | may be we will have 10/15 mins sync in a day or two | 15:36 |
| abhishekk | s/will/can | 15:37 |
| dansmith | sure | 15:38 |
| abhishekk | cool, thank you | 15:39 |
| abhishekk | I will add you as a reviewer to important specs | 15:39 |
| dansmith | xk | 15:44 |
| dansmith | er, ack even :) | 15:44 |
| * abhishekk signing out for the day | 16:23 | |
| *** tosky is now known as Guest7167 | 20:17 | |
| *** tosky_ is now known as tosky | 20:17 | |
| *** tosky_ is now known as tosky | 21:57 | |
Generated by irclog2html.py 2.17.2 by Marius Gedminas - find it at https://mg.pov.lt/irclog2html/!