Monday, 2021-11-08

*** gouthamr_ is now known as gouthamr04:07
mbeas"maaritamm", I was trying to make afix for the bug assigned 09:07
mbeashttps://bugs.launchpad.net/python-manilaclient/+bug/194909109:07
mbeas"maaritamm", now it is working locally09:08
maaritammmbeas, o/09:09
maaritammmbeas, awesome! :)09:09
mbeas"maaritamm", thank you for your help09:10
maaritammsure, np :) do we have a gerrit patch already?09:11
mbeasI will go and make it now and send here the url09:12
maaritammmbeas, ack, great!09:12
mbeas"maaritamm", The pep8 test also was successful09:14
maaritammcool! Did you also add a test for the new parameters?09:23
mbeas"maaritamm", I just made some tests locally. What should I do else?09:24
maaritammmbeas, ideally we'd like to add a unit test as well. I'll have to see if we have some examples for that somewhere. For now, you can push the patch to Gerrit and we'll go from there.09:28
mbeas"maaritamm", you are right. Your help would be much appreciated. I would like to work on the test too. It would be ideal.09:29
maaritammmbeas, sure :) you can add me as a reviewer on the patch so I get a notification. I'll try to take a look at it today, test it in my env. I didn't find any examples for pattern matching tests at the moment so we might need to get creative but you can look at the other tests for the same command and familiarise yourself with those to get the general idea: 09:36
maaritammhttps://github.com/openstack/python-manilaclient/blob/master/manilaclient/tests/unit/osc/v2/test_share.py#L47609:36
mbeas"maaritamm", at the examples you had provided in the share_snapshots.py file there wer some checks for api_versions. I included those too but I am not very sure for their usage.09:37
maaritammRight, that's a good point. Let's see ...09:38
maaritammmbeas, let's look at the API ref: https://docs.openstack.org/api-ref/shared-file-system/?expanded=list-shares-detail#list-shares09:39
maaritammLooks like those parameters were added to listing shares in microversions 2.3609:40
franciembeas, welcome. I too am writing a unit test for my patch and would like to help09:41
francieGood morning Everyone09:41
maaritammmbeas, 2.36 is the same microversion, so yes, you'll need to add that logic to your patch as well, even with the matching version :)09:42
maaritammfrancie, good morning!09:43
francieWelcome maaritanm09:43
mbeas"francie", good morning. Thank you, too -)09:44
mbeas"maaritamm", thank you for the link09:45
mbeas"maaritamm", https://review.opendev.org/c/openstack/python-manilaclient/+/81697609:57
maaritammmbeas, nice! The code looks good! I have some initial notes about the deleted comments though. The TODO comment, please still keep it there just delete the name~ and description~ , we still need to add --export-location support. I didn't include that in the bug report just to keep this change smaller. If you wish later, you can report that as a separate bug and add the support in a separate patch as well. The NOTE 10:08
maaritammcomment, please revert deleting it, it's meant to explain why we are using oscutils.sort_items before we return the data :)10:08
mbeas"maaritamm", the comment on lines 564 - 565?10:23
mbeas"maaritamm", sorry. I didn`t see the comment above your last comment -) (The big one)10:28
maaritammmbeas, oh, this channel is logged as most are so you can always refer back to that: https://meetings.opendev.org/irclogs/%23openstack-outreachy/latest.log.html 10:41
maaritammmbeas, and yes the one on lines 564-565, let's add that back10:41
mbeas"maaritamm", I reverted the todo and notes with little change10:43
maaritammmbeas, great, thank you! :)10:46
mbeas"maaritamm", I will now try to work on the test and then report the adding --export-location support as a bug and work on that 10:46
mbeas"maaritamm", thank you too -)10:47
maaritammmbeas, ok great, let me know if you have any questions10:47

Generated by irclog2html.py 2.17.2 by Marius Gedminas - find it at https://mg.pov.lt/irclog2html/!