Wednesday, 2022-06-22

*** dviroel|afk is now known as dviroel00:07
*** rlandy is now known as rlandy|out01:42
*** ysandeep|out is now known as ysandeep02:09
*** ysandeep is now known as ysandeep|afk07:02
*** ysandeep|afk is now known as ysandeep09:09
jm1sshnaidm, rcastillo: added a rfe based on an idea from a former colleague https://storyboard.openstack.org/#!/story/201010209:20
gtemajm1, rcastillo, sshnaidm: what do you think of https://review.opendev.org/c/openstack/ansible-collections-openstack/+/845782 + https://review.opendev.org/c/openstack/openstacksdk/+/845776. Should we go this way?09:20
jm1gtema: your change to sdk's cloud function is good for us. the aoc patch has to be tweaked but you dont have to do it because arxcruz already has a patch up for project_info :)09:34
gtemaI have seen it, and I actually pretty much reused it. It is just that I query SDK slightly differently09:36
jm1gtema: so you decided against a higher order function for filtering in resource.list? ^^ https://review.opendev.org/c/openstack/openstacksdk/+/84358709:36
gtemaif you are ok I will proceed with SDK change09:36
jm1gtema: sure09:36
jm1gtema: thanks!09:36
gtemathis is exactly the high-order implementation, just in the different place (due to architecture)09:37
gtemathis way it is usable by both cloud and proxy layer without issues and I also added proper tests for that09:38
jm1gtema: what higher-order impl do you mean? link?09:42
gtemahttps://review.opendev.org/c/openstack/openstacksdk/+/84572609:42
jm1gtema: ah nice. did not see it because its not merged yet ^^09:43
gtemait is set as dependency to my project_info change09:44
jm1nope, 845776 is a dependency of 845782. but 845726 is not. 10:06
jm1gtema: ah indirect dependency10:07
gtemadirect, but chained10:08
jm1gtema: ok, whatever this is called 😄 anyway, looks good!10:08
gtemaactually dependency on https://review.opendev.org/c/openstack/openstacksdk/+/845776 is not required itself, only 845726 is required10:08
gtemacool10:09
opendevreviewArtem Goncharov proposed openstack/ansible-collections-openstack master: Add SDK logging option for openstack ansible collections.  https://review.opendev.org/c/openstack/ansible-collections-openstack/+/84455910:13
jm1dtantsur, rcastillo: please rebase patches on top of master before merging them into aoc. this helps with tracking changes in master and helps with backporting patches to stable/1.0.010:13
dtantsurjm1: sorry, not following10:14
jm1dtantsur: by default zuul / gerrit will create a new merge commit if a patch cannot be fast-forwarded. this makes it harder to read the git history. e.g. it is harder to compare both branches (master and stable/1.0.0) and see what has to be backported etc10:17
jm1dtantsur: our current approach is to keep both branches similar. so stable/1.0.0 is like master but without breaking changes10:18
jm1dtantsur: we do this because openstacksdk<0.99.0 is not going away for a long time and hence our stable/1.0.0 wont be eol for a long time as well10:19
dtantsurThat's an impressive attempt, but if you don't automated it, it's not going work long-term10:20
dtantsurand if you do automate it, there is no point to put more cognitive load on the reviewers (and somewhat more load on zuul)10:21
dtantsurif you start tagging breaking changes in commit messages, you can even have a full automation with not so many Python lines of code10:21
dtantsurjust saying10:21
dtantsuralso nothing guarantees you that two changes won't be approved at the same time. and nothing guarantees you that the changes to stable merge in the same order. this is a windmill fighting.10:23
jm1dtantsur: automate what? zuul does not support automatic rebase. cherry-pick comes close but its still not the same. http://lists.zuul-ci.org/pipermail/zuul-discuss/2019-January/000694.html10:24
dtantsuryou don't need automatic rebase10:24
dtantsurthat's the point I'm making: you're choosing the wrong tool10:24
dtantsurwhat you need to know is what changes landed in master that did not land in stable/1.0. this is a not so difficult Python script.10:24
dtantsurif you tag breaking commits, you can exclude them and make the tool propose cherry-picks automatically.10:25
jm1dtantsur: regarding garantees about two approvals at the same time: this worked out till two weeks because we had only three workflowers, sshnaidm, rcastillo and me. we work closely together hence it worked fine.10:28
dtantsurjm1: so, you expect the reviews to never scale?10:28
dtantsur(probably true, but kinda pessimistic)10:29
jm1dtantsur: realistic? 10:29
dtantsurIf to make an approval here, I need to rebase the patch AND then come to IRC to coordinate AND make sure I only approve one patch at a time.. given how busy I am, what do you think will happen?10:30
dtantsur(I'm cool if you only ping me for baremetal reviews btw, I'll just drop aoc from my dashboard)10:34
*** rlandy__ is now known as rlandy10:34
jm1dtantsur: regarding the tool: most commits in master are breaking changes. at some point we have to serialize/synchronize in order to compare master and stable/1.0.0 and see what has been backported and what is different. i am open for new ideas how to improve that but the best thing we came up with so far is a hackmd where we track/coordinate our patches and git history for verification (of our hackmd etc.)10:37
dtantsur*shrug* I've told you my idea. you're welcome to do the same thing manually, of course, but just imagine how harder it will become with time as the number of commits to the master grows.10:39
jm1dtantsur: wrt minimizing your workload: you could +2 a patch and then ping us or wait for us to +w it. we (rcastillo and me) are tracking all aoc patches. it takes a while before you will get feedback because its a looot to do, but you will get it :)10:42
dtantsurack10:42
jm1dtantsur: since we are at it, what do you think about backporting our patches to stable/1.0.0? is it worth the effort? or do you think we should focus on master and only backport "on-demand" to stable/1.0.0?10:45
dtantsurjm1: with my stable core hat on and given the team's capacity, I think only important fixes should be backported.10:46
jm1sshnaidm: ^10:47
dtantsurkeep in mind: the code base will diverge quickly10:47
dtantsursomething that is an easy cherry-pick now, will be a merge nightmare in 2 months10:47
dtantsurespecially if, as you say, most of master commits are breaking10:48
jm1dtantsur: hmm.. it would really help to know how users are using our collection. if they can "simply" switch to a newer sdk release, then we could eol the stable/1.0.0 soon. but if we really want to support both branches for a long time, then it would probably make more sense to keep them as similar as possible, aka backport immediately10:51
gtemaI think users would love to switch to latest asap. It is just that at the moment they can't because it is not working everywhere10:52
gtemafrom my pov this is a bit of not required effort10:52
sshnaidm_I don't think we need to invest in backporting, I didn't even want to start backports :) Except of critical bug fixes there is no point to keep it up to date, it will be obsolete anyway when 2.0.0 is out11:05
*** sshnaidm_ is now known as sshnaidm11:05
sshnaidmAnd usually there are no critical bugfixes11:06
sshnaidmI think would be much more timeworthy to focus on new modules11:07
sshnaidms/new modules/modules for new sdk/11:07
sshnaidmI'd agree with dtantsur about rebases, if there is a request to change the Zuul/Gerrit default behavior it should be request for infra team. Asking everyone to rebase won't work.11:09
dtantsurjm1: it's a valid qustion, and a topic for a lot of potential reflection11:20
dtantsurif users do `pip install openstacksdk` or indirectly rely on upper-constraints, they'll get the new one11:20
opendevreviewAnanya proposed openstack/ansible-collections-openstack master: Makes security group rule info compatible with new sdk version  https://review.opendev.org/c/openstack/ansible-collections-openstack/+/84614811:25
*** dviroel__ is now known as dviroel11:28
gtemadtantsur: for the AOC usecase you will have the whole variety of ways to install sdk, but a regular user will only do `pip install openstacksdk`11:32
gtemaregular means "not operator"11:32
jm1dtantsur, gtema, sshnaidm: ack, thanks for your input! then i will stop backporting all fixes and only do it if necessary :)11:40
gtemacool11:40
jm1rcastillo: ^11:53
*** ysandeep is now known as ysandeep|afk12:23
opendevreviewAnanya proposed openstack/ansible-collections-openstack master: Makes security group rule info compatible with new sdk version  https://review.opendev.org/c/openstack/ansible-collections-openstack/+/84614812:30
*** ysandeep|afk is now known as ysandeep13:22
*** dviroel is now known as dviroel|lunch14:59
*** ysandeep is now known as ysandeep|out15:00
*** dviroel|lunch is now known as dviroel16:31
opendevreviewAnanya proposed openstack/ansible-collections-openstack master: Makes security group rule info compatible with new sdk version  https://review.opendev.org/c/openstack/ansible-collections-openstack/+/84614816:41
*** dviroel is now known as dviroel|afk19:48
*** dviroel|afk is now known as dviroel20:43
*** dviroel is now known as dviroel|afk21:22
*** rlandy is now known as rlandy|bbl22:30

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