*** dviroel|afk is now known as dviroel | 00:07 | |
*** rlandy is now known as rlandy|out | 01:42 | |
*** ysandeep|out is now known as ysandeep | 02:09 | |
*** ysandeep is now known as ysandeep|afk | 07:02 | |
*** ysandeep|afk is now known as ysandeep | 09:09 | |
jm1 | sshnaidm, rcastillo: added a rfe based on an idea from a former colleague https://storyboard.openstack.org/#!/story/2010102 | 09:20 |
---|---|---|
gtema | jm1, 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 |
jm1 | gtema: 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 |
gtema | I have seen it, and I actually pretty much reused it. It is just that I query SDK slightly differently | 09:36 |
jm1 | gtema: so you decided against a higher order function for filtering in resource.list? ^^ https://review.opendev.org/c/openstack/openstacksdk/+/843587 | 09:36 |
gtema | if you are ok I will proceed with SDK change | 09:36 |
jm1 | gtema: sure | 09:36 |
jm1 | gtema: thanks! | 09:36 |
gtema | this is exactly the high-order implementation, just in the different place (due to architecture) | 09:37 |
gtema | this way it is usable by both cloud and proxy layer without issues and I also added proper tests for that | 09:38 |
jm1 | gtema: what higher-order impl do you mean? link? | 09:42 |
gtema | https://review.opendev.org/c/openstack/openstacksdk/+/845726 | 09:42 |
jm1 | gtema: ah nice. did not see it because its not merged yet ^^ | 09:43 |
gtema | it is set as dependency to my project_info change | 09:44 |
jm1 | nope, 845776 is a dependency of 845782. but 845726 is not. | 10:06 |
jm1 | gtema: ah indirect dependency | 10:07 |
gtema | direct, but chained | 10:08 |
jm1 | gtema: ok, whatever this is called 😄 anyway, looks good! | 10:08 |
gtema | actually dependency on https://review.opendev.org/c/openstack/openstacksdk/+/845776 is not required itself, only 845726 is required | 10:08 |
gtema | cool | 10:09 |
opendevreview | Artem Goncharov proposed openstack/ansible-collections-openstack master: Add SDK logging option for openstack ansible collections. https://review.opendev.org/c/openstack/ansible-collections-openstack/+/844559 | 10:13 |
jm1 | dtantsur, 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.0 | 10:13 |
dtantsur | jm1: sorry, not following | 10:14 |
jm1 | dtantsur: 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 etc | 10:17 |
jm1 | dtantsur: our current approach is to keep both branches similar. so stable/1.0.0 is like master but without breaking changes | 10:18 |
jm1 | dtantsur: 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 well | 10:19 |
dtantsur | That's an impressive attempt, but if you don't automated it, it's not going work long-term | 10:20 |
dtantsur | and 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 |
dtantsur | if you start tagging breaking changes in commit messages, you can even have a full automation with not so many Python lines of code | 10:21 |
dtantsur | just saying | 10:21 |
dtantsur | also 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 |
jm1 | dtantsur: 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.html | 10:24 |
dtantsur | you don't need automatic rebase | 10:24 |
dtantsur | that's the point I'm making: you're choosing the wrong tool | 10:24 |
dtantsur | what 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 |
dtantsur | if you tag breaking commits, you can exclude them and make the tool propose cherry-picks automatically. | 10:25 |
jm1 | dtantsur: 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 |
dtantsur | jm1: so, you expect the reviews to never scale? | 10:28 |
dtantsur | (probably true, but kinda pessimistic) | 10:29 |
jm1 | dtantsur: realistic? | 10:29 |
dtantsur | If 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 rlandy | 10:34 | |
jm1 | dtantsur: 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 |
jm1 | dtantsur: 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 |
dtantsur | ack | 10:42 |
jm1 | dtantsur: 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 |
dtantsur | jm1: with my stable core hat on and given the team's capacity, I think only important fixes should be backported. | 10:46 |
jm1 | sshnaidm: ^ | 10:47 |
dtantsur | keep in mind: the code base will diverge quickly | 10:47 |
dtantsur | something that is an easy cherry-pick now, will be a merge nightmare in 2 months | 10:47 |
dtantsur | especially if, as you say, most of master commits are breaking | 10:48 |
jm1 | dtantsur: 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 immediately | 10:51 |
gtema | I think users would love to switch to latest asap. It is just that at the moment they can't because it is not working everywhere | 10:52 |
gtema | from my pov this is a bit of not required effort | 10: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 out | 11:05 |
*** sshnaidm_ is now known as sshnaidm | 11:05 | |
sshnaidm | And usually there are no critical bugfixes | 11:06 |
sshnaidm | I think would be much more timeworthy to focus on new modules | 11:07 |
sshnaidm | s/new modules/modules for new sdk/ | 11:07 |
sshnaidm | I'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 |
dtantsur | jm1: it's a valid qustion, and a topic for a lot of potential reflection | 11:20 |
dtantsur | if users do `pip install openstacksdk` or indirectly rely on upper-constraints, they'll get the new one | 11:20 |
opendevreview | Ananya 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/+/846148 | 11:25 |
*** dviroel__ is now known as dviroel | 11:28 | |
gtema | dtantsur: 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 |
gtema | regular means "not operator" | 11:32 |
jm1 | dtantsur, gtema, sshnaidm: ack, thanks for your input! then i will stop backporting all fixes and only do it if necessary :) | 11:40 |
gtema | cool | 11:40 |
jm1 | rcastillo: ^ | 11:53 |
*** ysandeep is now known as ysandeep|afk | 12:23 | |
opendevreview | Ananya 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/+/846148 | 12:30 |
*** ysandeep|afk is now known as ysandeep | 13:22 | |
*** dviroel is now known as dviroel|lunch | 14:59 | |
*** ysandeep is now known as ysandeep|out | 15:00 | |
*** dviroel|lunch is now known as dviroel | 16:31 | |
opendevreview | Ananya 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/+/846148 | 16:41 |
*** dviroel is now known as dviroel|afk | 19:48 | |
*** dviroel|afk is now known as dviroel | 20:43 | |
*** dviroel is now known as dviroel|afk | 21:22 | |
*** rlandy is now known as rlandy|bbl | 22:30 |
Generated by irclog2html.py 2.17.3 by Marius Gedminas - find it at https://mg.pov.lt/irclog2html/!