| *** hamalq has quit IRC | 00:13 | |
| corvus | clarkb, ianw: left some comments on 787475; sorry i have to run to dinner now. | 01:17 |
|---|---|---|
| *** ajitha has joined #zuul | 02:09 | |
| *** evrardjp has quit IRC | 02:33 | |
| *** evrardjp has joined #zuul | 02:33 | |
| *** rlandy|rover|bbl has quit IRC | 03:42 | |
| *** ikhan has joined #zuul | 03:44 | |
| *** ikhan has quit IRC | 03:49 | |
| *** bhavikdbavishi has joined #zuul | 04:08 | |
| *** bhavikdbavishi has quit IRC | 04:12 | |
| *** ykarel|away has joined #zuul | 04:18 | |
| *** ajitha has quit IRC | 04:18 | |
| *** ikhan has joined #zuul | 04:31 | |
| *** vishalmanchanda has joined #zuul | 04:36 | |
| *** jfoufas1 has joined #zuul | 04:55 | |
| *** ykarel_ has joined #zuul | 04:55 | |
| *** ykarel|away has quit IRC | 04:58 | |
| ianw | corvus: thanks, i'm going to have to look at that with fresher eyes :) but will next week | 05:13 |
| *** ykarel__ has joined #zuul | 05:23 | |
| *** ykarel_ has quit IRC | 05:23 | |
| *** ykarel_ has joined #zuul | 05:41 | |
| *** ykarel_ is now known as ykarel | 05:43 | |
| *** ykarel__ has quit IRC | 05:43 | |
| *** jcapitao has joined #zuul | 06:22 | |
| *** LiangLu has joined #zuul | 06:48 | |
| *** saneax has joined #zuul | 07:03 | |
| *** ykarel has quit IRC | 07:07 | |
| *** ykarel has joined #zuul | 07:10 | |
| *** rpittau|afk is now known as rpittau | 07:12 | |
| *** ykarel_ has joined #zuul | 07:12 | |
| *** nils has joined #zuul | 07:14 | |
| *** ykarel has quit IRC | 07:15 | |
| openstackgerrit | Albin Vass proposed zuul/zuul-jobs master: Roles to create, cleanup and promote snapshots in ec2 https://review.opendev.org/c/zuul/zuul-jobs/+/787677 | 07:41 |
| *** ykarel_ has quit IRC | 07:45 | |
| *** shanemcd has quit IRC | 07:50 | |
| *** shanemcd has joined #zuul | 07:50 | |
| *** tosky has joined #zuul | 07:53 | |
| *** jpena|off is now known as jpena | 07:54 | |
| *** ykarel has joined #zuul | 07:55 | |
| *** harrymichal has joined #zuul | 08:04 | |
| *** ykarel has quit IRC | 08:04 | |
| *** harrymichal has quit IRC | 08:05 | |
| *** harrymichal has joined #zuul | 08:05 | |
| *** ykarel has joined #zuul | 08:06 | |
| openstackgerrit | Felix Edel proposed zuul/zuul master: Execute builds via ZooKeeper https://review.opendev.org/c/zuul/zuul/+/770902 | 08:22 |
| openstackgerrit | Felix Edel proposed zuul/zuul master: Fix test_gerrit.TestPolling.test_config_update https://review.opendev.org/c/zuul/zuul/+/773023 | 08:22 |
| openstackgerrit | Felix Edel proposed zuul/zuul master: Switch to ZooKeeper backed merge result events https://review.opendev.org/c/zuul/zuul/+/784195 | 08:22 |
| openstackgerrit | Felix Edel proposed zuul/zuul master: Improve component registry https://review.opendev.org/c/zuul/zuul/+/787684 | 08:22 |
| openstackgerrit | Felix Edel proposed zuul/zuul master: Calculate statsd metrics via ComponentRegistry https://review.opendev.org/c/zuul/zuul/+/787685 | 08:22 |
| openstackgerrit | Felix Edel proposed zuul/zuul master: Execute merge jobs via ZooKeeper https://review.opendev.org/c/zuul/zuul/+/787686 | 08:22 |
| LiangLu | Hi there, this is Liang from tacker team, we need to create 2-NIC nodes during Zuul FT test job, we tried to set label of node, but seems only rax provider will create 2-NIC controller node, do you have any experience of that? | 08:31 |
| *** saneax has quit IRC | 08:33 | |
| *** harrymichal has quit IRC | 08:34 | |
| lyr | Hey. I've this error when trying to add an autohold through zuul-client https://paste.garrigue.re/?99009e4216471254#DujC4FvXJisGZRAkndsFkvVb6wdXBdDA5dN22koxyixh | 08:59 |
| lyr | An ugly jwt "not enough segment" | 09:00 |
| lyr | autohold-list is running fine | 09:05 |
| lyr | running the equivalent command with zuul cli directly on the server works | 09:06 |
| mhu | lyr: how did you get your token? | 09:06 |
| lyr | through the UI | 09:07 |
| lyr | url https://softwarefactory/sf/user_settings.html | 09:07 |
| mhu | lyr: that's not the right token, sorry. I guess we should clarify this in SF's documentation | 09:08 |
| mhu | at the moment you need to generate the token with the zuul admin CLI on the server | 09:08 |
| lyr | I guess "zuul create-auth-token" is the way to go ? | 09:08 |
| mhu | check "zuul create-auth-token --help" | 09:08 |
| mhu | yes exactly | 09:08 |
| lyr | is there a way to get a token form the UI ? | 09:09 |
| lyr | (for my developers' internal doc) | 09:09 |
| mhu | not at the moment, but there's a big patch chain to add support for the admin commands in the GUI | 09:09 |
| mhu | this also adds a user page where you can get your token when authenticated | 09:09 |
| mhu | this way you can also use the CLI | 09:10 |
| lyr | ok it works now, thanks mhu. Do you have an idea which SF release this user page feature will be released in ? 3.7 ? | 09:20 |
| mhu | it needs to land upstream first | 09:20 |
| mhu | I'm back from paternity leave so I can drive this effort again | 09:20 |
| mhu | once we get the feature in zuul, SF will also need to switch to a more modern SSO for jwt support - we plan to replace the current SSO with keycloak | 09:21 |
| mhu | so there's a lot to do ... but it's on the roadmap | 09:22 |
| *** ikhan has quit IRC | 09:37 | |
| *** ikhan has joined #zuul | 09:37 | |
| *** ykarel_ has joined #zuul | 09:39 | |
| *** ykarel has quit IRC | 09:41 | |
| *** ykarel_ is now known as ykarel | 09:42 | |
| lyr | I saw keycloack on the SF 4 roadmap | 09:57 |
| lyr | Thanks for the intel | 09:57 |
| mhu | yw | 09:57 |
| *** ikhan has quit IRC | 10:12 | |
| *** ikhan has joined #zuul | 10:12 | |
| *** ykarel_ has joined #zuul | 10:15 | |
| *** ikhan has quit IRC | 10:17 | |
| *** ykarel has quit IRC | 10:18 | |
| lyr | mhu the create-auth-token is temporary ? | 10:19 |
| lyr | Got a "Unauthorized - your token might be invalid or expired." | 10:19 |
| mhu | lyr: yes, check the --help, there's an option to set the expiry | 10:19 |
| *** jcapitao has quit IRC | 10:24 | |
| *** ikhan has joined #zuul | 10:27 | |
| *** ykarel__ has joined #zuul | 10:31 | |
| *** ikhan has quit IRC | 10:31 | |
| *** ykarel_ has quit IRC | 10:33 | |
| *** ykarel_ has joined #zuul | 10:36 | |
| *** ykarel_ is now known as ykarel | 10:37 | |
| *** ykarel__ has quit IRC | 10:39 | |
| *** ikhan has joined #zuul | 10:43 | |
| *** ikhan has quit IRC | 11:27 | |
| *** jpena is now known as jpena|lunch | 11:30 | |
| *** rlandy has joined #zuul | 11:42 | |
| *** rlandy is now known as rlandy|rover | 11:43 | |
| *** jpena|lunch is now known as jpena | 12:33 | |
| *** mgoddard has joined #zuul | 12:44 | |
| openstackgerrit | Albin Vass proposed zuul/zuul-jobs master: Roles to create, cleanup and promote snapshots in ec2 https://review.opendev.org/c/zuul/zuul-jobs/+/787677 | 12:48 |
| *** vishalmanchanda has quit IRC | 13:26 | |
| *** ykarel_ has joined #zuul | 13:36 | |
| *** ykarel has quit IRC | 13:39 | |
| *** ssmashnuk has joined #zuul | 13:39 | |
| *** y2kenny has joined #zuul | 13:40 | |
| ssmashnuk | Good morning everyone. Zuul newby here looking to use some static WIN2019 nodes, but I'm getting the following error: "msg": "winrm or requests is not installed: No module named 'winrm'". Can someone point me in the right direction? | 13:44 |
| avass | ssmashnuk: you probably need to install pywinrm on the executors :) | 13:48 |
| corvus | or i guess specifically that needs to be in the virtualenv the executors use to run ansible? | 13:49 |
| avass | ssmashnuk: https://docs.ansible.com/ansible/latest/user_guide/windows_winrm.html#what-is-winrm | 13:49 |
| avass | corvus: yeah | 13:49 |
| avass | I think there's a environment variable to handle that | 13:49 |
| corvus | we should probably install that by default and include it in our images? | 13:50 |
| avass | ssmashnuk: ANSIBLE_EXTRA_PACKAGES https://zuul-ci.org/docs/zuul/howtos/installation.html?highlight=extra_packages#ansible | 13:50 |
| avass | corvus: probably a good idea, that ^ variables wasn't well documented what I could see either | 13:51 |
| ssmashnuk | Great, thanks corvus, I'll give it a try. | 13:52 |
| ssmashnuk | As a note, I do have winrm installed on my executors and using the ansible_python I can import winrm and connect. | 13:53 |
| avass | Is pywinrm installed on the system or in the ansible virtualenvs? | 13:54 |
| corvus | (zuul executors use zuul-manage-ansible to make a virtualenv for each of the [currently 2] versions of ansible they support) | 13:55 |
| ssmashnuk | System, which is probably the issue. | 13:55 |
| corvus | cool, so the link avass pointed you to should take care of it | 13:55 |
| avass | ssmashnuk: also you could try using openssh as well, winrm can't really handle copying files very well so the preferred way to synchronize repositories from the executor is to use ssh in some way anyway | 13:55 |
| avass | but you'd need to install ssh on the windows nodes in that case | 13:56 |
| openstackgerrit | James E. Blair proposed zuul/zuul master: Add winrm to default executor venv packages https://review.opendev.org/c/zuul/zuul/+/787743 | 13:57 |
| avass | corvus: I think it's pywinrm | 13:58 |
| avass | oh looks like it's in pypi with both names | 13:59 |
| corvus | different maintainers though | 13:59 |
| avass | yeah | 13:59 |
| corvus | so it's either a helpful attempt to alias the name, or a trojan | 14:00 |
| corvus | hard to say which | 14:00 |
| openstackgerrit | James E. Blair proposed zuul/zuul master: Add pywinrm to default executor venv packages https://review.opendev.org/c/zuul/zuul/+/787743 | 14:00 |
| openstackgerrit | Albin Vass proposed zuul/zuul master: Add pywinrm to default executor venv packages https://review.opendev.org/c/zuul/zuul/+/787751 | 14:13 |
| avass | oops bad copy/paste :) | 14:13 |
| openstackgerrit | Albin Vass proposed zuul/zuul master: Add boto3 to default executor venv packages https://review.opendev.org/c/zuul/zuul/+/787751 | 14:13 |
| avass | (to make it easier to use upload-logs-s3 and snapshot instances) | 14:14 |
| corvus | ++ | 14:14 |
| *** rpittau is now known as rpittau|afk | 14:15 | |
| *** ykarel_ is now known as ykarel | 14:26 | |
| lyr | I hitted a lot of NODE_FAILURE on a batch of change across a lot of repoisotires, since my openstack was full (not enough quota). I'm supprised the job didn't waited. Is there something like a waiting timeout I can set somewhere ? | 14:35 |
| clarkb | lyr: you can set max-servers on the pool, but also nodepool will respect instance, cpu, and memory quotas if you set them in the cloud | 14:36 |
| lyr | I did set max-servers. That's why I got a "not enough quota", it's fine. But I expected the jobs to wait for a node | 14:37 |
| clarkb | lyr: if you set max-servers then nodepool will hit that limit and wait before trying to boot more instances | 14:38 |
| clarkb | having NODE_FAILURES implies that you were not yet to max-servers and started to fail | 14:38 |
| clarkb | and then those failures got reported back as NODE_FAILURE | 14:38 |
| lyr | Just to be clear, by NODE_FAILURE, I mean https://i.imgur.com/OKTdUH0.png | 14:39 |
| corvus | lyr: do you have logs from the launcher indicating the cause? | 14:40 |
| clarkb | lyr: yes, roughly the way nodepool works is: check if I have room according to max-servers or quota, if there is room then boot, if boot fails three times in a row report NODE_FAILURE, if there is no room, stop processing node requests and wait until there is room | 14:40 |
| clarkb | my read on that is that the boots failed while you were below max-servers and quota limits (which can happen) | 14:41 |
| lyr | smell like our old openstack being crancky https://paste.garrigue.re/?bb1a96be7d200c6d#FZQFg4u3me1dauiQZf6N22WQnmYxWowD7ZSdbDWZTPDg | 14:41 |
| lyr | Hmm sorry for bothering, it seems there have been a glitch, now it seems to be working | 14:44 |
| *** ykarel is now known as ykarel|away | 14:50 | |
| *** ykarel|away has quit IRC | 14:59 | |
| *** hashar has joined #zuul | 15:03 | |
| openstackgerrit | Matthieu Huin proposed zuul/zuul-client master: [WIP] add tenant as a config option https://review.opendev.org/c/zuul/zuul-client/+/787770 | 15:03 |
| *** hashar has quit IRC | 15:10 | |
| *** hashar has joined #zuul | 15:10 | |
| *** hashar has quit IRC | 15:10 | |
| *** hashar_ has joined #zuul | 15:10 | |
| openstackgerrit | Albin Vass proposed zuul/zuul-jobs master: intercept-job -- self-service SSH access https://review.opendev.org/c/zuul/zuul-jobs/+/679306 | 15:14 |
| *** Shrews has joined #zuul | 15:37 | |
| openstackgerrit | Merged zuul/zuul-jobs master: intercept-job -- self-service SSH access https://review.opendev.org/c/zuul/zuul-jobs/+/679306 | 15:42 |
| *** hashar_ has quit IRC | 15:53 | |
| *** nils has quit IRC | 16:13 | |
| *** hamalq has joined #zuul | 16:27 | |
| *** hamalq_ has joined #zuul | 16:33 | |
| *** hamalq has quit IRC | 16:36 | |
| *** jpena is now known as jpena|off | 16:53 | |
| openstackgerrit | James E. Blair proposed zuul/zuul master: Switch to ZooKeeper backed build result events https://review.opendev.org/c/zuul/zuul/+/782939 | 16:54 |
| avass | corvus: speaking of NODE_FAILURES, I sometimes get boot timeouts for my digital ocean drivers that is using the statemachine driver. it looks like the provider doesn't retry if the boot times out and most of the boots (like at least 9/10) works so wouldn't it be better to retry if the timeout is exceeded? | 17:10 |
| avass | I mean retry up to 3 tries or something | 17:10 |
| fungi | in the openstack provider driver i see launch attempts retried after boot timeouts (up to 3 times before the launcher declines the request) | 17:11 |
| avass | so maybe that's something that should be done in the digital ocean driver or in the statemachine driver I suppose | 17:12 |
| avass | statemachine framework* | 17:12 |
| corvus | avass: yeah, due to the way the nodepool launcher is written, it's not really feasible for the state machine driver to handle that, so for azure, i put it inside the state machine itself: https://review.opendev.org/780682 | 17:12 |
| corvus | avass: i would like to correct that, but it's a large change to nodepool | 17:12 |
| avass | corvus: ok then I know to implement that in the driver at least | 17:13 |
| corvus | yeah, that should work; i don't like it because it makes for a complicated state machine, and it's really just handling one thing that could go wrong, whereas if we generalize it, it could transparently handle more kinds of failures. | 17:14 |
| corvus | avass: but i think that's what we have to do for now until we can revisit it. should be easy to pull that out of the statemachine drivers once we can. | 17:15 |
| avass | yup | 17:15 |
| avass | I think the ec2 driver could be suffering from that as well, but we've put a long boot timeout for now to get around it | 17:16 |
| y2kenny | In Zuul web UI for build result, each build is listed to show "change" which is <change id>,<patch id> for Gerrit and the change can be use for filtering. Is there a possibility to configure zuul to display the change's git short hash as well? (for both display and filtering?) | 17:25 |
| corvus | y2kenny: that is not currently supported (and i don't think that info is currently stored in the db) | 17:29 |
| y2kenny | corvus: has there been similar request in the past? is the feature worth exploring? | 17:31 |
| corvus | y2kenny: to my knowledge, you're the first; and i don't see why not :) | 17:34 |
| y2kenny | corvus: ok. I just figure the git hash is potentially more universal in terms of finding things. I am guessing for GitHub based job the pull request number shows up there instead? | 17:35 |
| *** jfoufas1 has quit IRC | 17:36 | |
| corvus | y2kenny: pr,sha for github | 17:36 |
| corvus | y2kenny: zuul doesn't really have any use for it; it's not a unique identifier for a change, and zuul deals with changes | 17:37 |
| corvus | but i could see a user wanting to see what builds have run for the change(s) associated with a commit | 17:38 |
| y2kenny | corvus: um... right. Ok, I will think about it some more. | 17:38 |
| corvus | so carrying it around as duplicate data for humans is fine; i just mention that to indicate why it's not a central piece of info for zuul | 17:38 |
| y2kenny | understood. | 17:38 |
| fungi | i suppose each source connection driver would need to decide how commits are associated with a change | 17:39 |
| fungi | for a github pull request, would it be any commits included in that pull request? | 17:40 |
| fungi | or only the "tip" commit? | 17:40 |
| corvus | fungi: typically the PR tip; that's what's used as the patchset identifier for the pr | 17:40 |
| avass | fungi: I think that's currently part of mqtt message and reports the tip | 17:41 |
| fungi | for a gerrit change, would it be only the patchset(s) which had that commit, or any patchset for the same change which had that commit as a patchset? | 17:41 |
| corvus | yeah, all the drivers have this info; it's just not used for changes | 17:41 |
| fungi | true, i suppose github pr revisions and gerrit change revisions both get new commit ids... though at least gerrit is able to have the same commit id on (non-sequential) patchsets for a change | 17:42 |
| corvus | in all cases there is a 1->N mapping of change+patchset -> commit. so it's possible for us to carry around the commit for a change, store it in the db, and do a reverse lookup on that commit to tell you all the jobs that ran for all the changes that pointed to that commit. | 17:43 |
| corvus | a change in zuul is a kind of ref, and a ref always points to a git sha | 17:44 |
| corvus | reversing that gets you multiple changes | 17:44 |
| corvus | (potentially, but rarely in practice) | 17:44 |
| corvus | on top of that, it's worth keeping in mind that the sha of the change's commit is rarely the repo HEAD that zuul tests (because it's usually testing synthetic merge commits) | 17:45 |
| corvus | but we can still say "this is the commit that we merged with a bunch of other commits on top of the branch which moved ahead since the commit was written" when we ran this job :) | 17:46 |
| corvus | we can guarantee that commit was somewhere in the dag | 17:46 |
| fungi | right, it's not so much "the commit that zuul tested" but rather "a convenient handle for what triggered some buildset" | 17:48 |
| corvus | exactly | 17:49 |
| fungi | (possible multiple buildsets) | 17:49 |
| *** LiangLu has quit IRC | 17:50 | |
| fungi | might represent buildsets from the same event in multiple pipelines, or rechecks of the same change, or less common collisions of that commit handle to different change revisions | 17:50 |
| fungi | that last is where the change identifier is more specific | 17:51 |
| *** cloudnull has quit IRC | 18:11 | |
| openstackgerrit | James E. Blair proposed zuul/zuul master: Switch to ZooKeeper backed build result events https://review.opendev.org/c/zuul/zuul/+/782939 | 18:33 |
| *** nils has joined #zuul | 18:42 | |
| *** nils has quit IRC | 18:52 | |
| -openstackstatus- NOTICE: The Gerrit service on review.openstack.org is being restarted to pick up some updates, and should be available again momentarily | 19:03 | |
| *** nils has joined #zuul | 19:04 | |
| *** Eighth_Doctor is now known as Conan_Kudo | 19:27 | |
| *** y2kenny has quit IRC | 19:28 | |
| *** Conan_Kudo is now known as Eighth_Doctor | 19:29 | |
| *** cloudnull has joined #zuul | 19:47 | |
| *** nils has quit IRC | 20:22 | |
| openstackgerrit | Matthieu Huin proposed zuul/zuul-client master: Add tenant as a config option https://review.opendev.org/c/zuul/zuul-client/+/787770 | 20:25 |
| mhu | lyr: ^ | 20:26 |
| clarkb | mhu: can I suggest that you reformat with black in a separate change? | 20:26 |
| clarkb | (also if the plan is to use black I would have it enforce that in the linter job) | 20:27 |
| mhu | clarkb, fair enough - I used black because all my cool friends do, I caved in to peer pressure | 20:29 |
| clarkb | I personally wouldn't mind using it either, but not sure about others. THe important thing though is to make that its own change imo | 20:29 |
| clarkb | and then be consistent about it if that is the format we want to use | 20:29 |
| clarkb | mixing the changes and then not enforcing it will just cause things to regress over time | 20:29 |
| pabelanger | I like black for enforcement, we are using it for ansible collections | 20:30 |
| pabelanger | but agree, adding it into linters jobs to gate should also be done | 20:30 |
| pabelanger | (if adopted) | 20:30 |
| mhu | pabelanger, do you have a job definition example using black I could look at? | 20:31 |
| clarkb | mhu: you would use the existing linters job definition but update tox.ini to use black instead of pep8/pycodestyle | 20:32 |
| fungi | to be honest i'm not a huge fan of adding another style checker which will cause a complete update of the entire codebase just to shuffle whitespace around. seems like there's more useful things to work on | 20:32 |
| clarkb | you can't use both as they do not agree on style | 20:32 |
| pabelanger | https://github.com/ansible-collections/ansible.utils/blob/main/tox.ini | 20:32 |
| pabelanger | we have tox -elinters / tox -eblack | 20:32 |
| clarkb | fungi: this is a new codebase at least | 20:32 |
| pabelanger | 1 check, 1 reformat | 20:32 |
| mhu | thanks pabelanger | 20:32 |
| * pabelanger goes back to the shadows | 20:33 | |
| fungi | clarkb: yep, but then you get the "this one repo uses a different coding style than the rest of the project" | 20:33 |
| clarkb | fungi: fair | 20:33 |
| fungi | becomes fun to explain to contributors why they need to use different coding style in different repos for the same project | 20:34 |
| mhu | ok I'll split this in two, and we can have a further discussion on adopting black on one of the reviews | 20:34 |
| corvus | we've already had that discussion | 20:34 |
| corvus | can we not have it again? | 20:35 |
| mhu | ok so I'll drop the changes pertaining to using black | 20:36 |
| mhu | I don't remember that discussion, but then again, I've been suffering a severe case of dad brains for the last month | 20:37 |
| clarkb | I think the previous discussion happened when black was very new and zuul's max python version was < black's min python version. That said at the time no one was super keen on using it | 20:38 |
| corvus | the short of it is that we try to avoid spending our time on whitespace and just follow the pep8 guide loosely, especially the first admonition. | 20:42 |
| * corvus goes back to not-whitespace things | 20:42 | |
| fungi | my take on coding style is "de gustibus non est disputandum" | 20:44 |
| clarkb | for me the reason I'm willing to contemplate black is it is intentionally designed to make diffs smaller and more readable for human reviewers. But I'm happy to let it be and not worry about it | 20:47 |
| avass | I'm still in favour of the not caring too much about formatting as long as the code is readable and sane. which would would be caught during review anyway | 20:49 |
| fungi | yep, i think a majority of programmers are capable of judging when they should break up array elements into additional lines for cleaner updates without a style checker preventing them from not doing it when it makes sense not to do that | 20:51 |
| avass | fungi: had to look that and yes :) | 20:51 |
| fungi | and if a diff is hard to review, please ask the author to do whatever is necessary to make it understandable | 20:53 |
| openstackgerrit | Matthieu Huin proposed zuul/zuul-client master: Add tenant as a config option https://review.opendev.org/c/zuul/zuul-client/+/787770 | 21:12 |
| *** Shrews has quit IRC | 21:13 | |
| corvus | mhu: quick suggestion on that | 21:16 |
| openstackgerrit | Matthieu Huin proposed zuul/zuul-client master: Add tenant as a config option https://review.opendev.org/c/zuul/zuul-client/+/787770 | 21:20 |
| mhu | corvus, thanks, fixed ^ | 21:20 |
| corvus | +2 | 21:23 |
| corvus | mhu: i ran into this issue the other day (i've been making sure to use zuul-client as much as possible as a user recently) -- i'd like your thoughts on it when you have a sec: https://review.opendev.org/787642 | 21:23 |
| corvus | i'm thinking the principle of least surprise is: strip on interactive use (because it's probably a password) and don't strip on files, because you just want the file to show up exactly as-is | 21:25 |
| mhu | corvus, that makes sense, if you're encrypting a file chances are you want it encrypted (and decrypted later) as is | 21:25 |
| mhu | was that something that didn't make the jump from the encrypting script in the zuul repo? | 21:27 |
| clarkb | corvus: ++ that behavior makes sense to me | 21:27 |
| corvus | mhu: i don't think so -- i thought the same thing and i went and looked, and i think the current behaviors match | 21:27 |
| fungi | huh, i thought we flipped the default on --strip | 21:32 |
| fungi | https://review.opendev.org/714508 last year | 21:33 |
| corvus | fungi: we did; this flips it back halfway :) | 21:34 |
| fungi | ahh, okay | 21:35 |
| corvus | there are a lot of files that will work fine without a trailing newline; i just happened to run into one that didn't, so i noticed this. | 21:36 |
| avass | I think vim stripping newlines for me saves me here. I usually dump a secret string to a file and encrypt that :) | 21:45 |
| fungi | oh, huh. for some reason i thought we'd left it off for files, on for stdin/pipe | 21:45 |
| avass | I'm fine with either but making sure that eventually reaches a stable state without changing the behaviour too often is probably good. | 21:46 |
| *** nils has joined #zuul | 21:48 | |
| corvus | fungi: that's what i thought; which is why i wrote the change to make it do what i thought it did :) | 21:48 |
| corvus | avass: indeed | 21:49 |
| corvus | apparently at least 3 of us thought that was the behavior | 21:49 |
| corvus | i also considered stripping if there is only one line of text; that would be almost foolproof except for a binary data file that happened to end with a \n. | 21:52 |
| corvus | but that seems like a harder behavior to explain | 21:52 |
| avass | yeah I just rely on fzf to have the command in history so I usually just create a file called something like: 'infile', Ctrl+R, enter. | 21:54 |
| *** rlandy|rover has quit IRC | 22:15 | |
| *** nils has quit IRC | 22:48 | |
| *** ssmashnuk has quit IRC | 23:23 | |
| *** tosky has quit IRC | 23:58 | |
Generated by irclog2html.py 2.17.2 by Marius Gedminas - find it at https://mg.pov.lt/irclog2html/!
