| GhostLyr_ | Greetings! I was wondering if openstack/etcd3gw is open for community contributions. There are several issues in the associated launchpad project but it seems nobody from the project ever interacted with reported bugs in any way, despite even patches being available for some of them. | 13:17 |
|---|---|---|
| GhostLyr_ | Meanwhile there have been commits to the git repo, so I'm assuming it's not dead or abandoned. | 13:17 |
| GhostLyr_ | Unfortunately the GitHub is a mirror, else I'd have sent issues and patches there | 13:17 |
| tkajinam | GhostLyr_, it's open but not very active. it's one of the many repository maintained by a very small team and unfortunately we don't have very enough bandwidth to react on all bug reports, especially just minor suggestions, without actual fix proposed | 14:01 |
| tkajinam | GhostLyr_, https://docs.openstack.org/contributors/code-and-documentation/index.html | 14:01 |
| tkajinam | Hmm we have to update that README in the repo. thanks for pointing that | 14:01 |
| GhostLyr_ | but, this does even proposed a fix: https://bugs.launchpad.net/python-etcd3gw/+bug/2012261 (ofc I don't know if that solves the issue, just an example) | 14:02 |
| GhostLyr_ | full disclosure: I'm also affected by this specific issue | 14:03 |
| opendevreview | Takashi Kajinami proposed openstack/etcd3gw master: Update README https://review.opendev.org/c/openstack/etcd3gw/+/908582 | 14:14 |
| tkajinam | that may be our fault, but it's quite difficult to monitor all of incoming bugs in 30+ projects and pick up any potential fixes proposed there, without actual code change submitted to the review system | 14:16 |
| GhostLyr_ | well… tbh I looked over your docs and the entry barrier compared to submit a quick MR/PR on github/gitlab is significantly higher… | 14:17 |
| tkajinam | I know but that's how the projects are kept in OpenStack namespace now | 14:17 |
| tkajinam | and it's now maintained here because it was initially launched for some use case within OpenStack | 14:17 |
| JayF | I know it's a little different, but at least we don't do the mailing list patch stuff 😆 | 14:18 |
| GhostLyr_ | :/ | 14:18 |
| tkajinam | you know how much I've been struggling with reading kernel patches recently after living long with gerrit :-) | 14:19 |
| JayF | I'll see if I can knock out that bug today though, it looks pretty straightforward | 14:19 |
| JayF | I'll offer you the opportunity if you have a patch if you want to post it on a pr, I'm happy to proxy it for you | 14:19 |
| GhostLyr_ | it's different enough that I worry i'm not the only one who will just consider forking the github mirror, plopping some commits of my own on top and then using that instead of the official distribution just because the feels less painful :( | 14:19 |
| JayF | Otherwise I will try to find time to do it today, but I'll warn you that that does not mean it will get done for sure | 14:19 |
| JayF | Yeah that's literally what I was saying, if you want to put a PR up via GitHub, as long as I review it and don't mind attaching my name to it I'm happy to proxy it for you and give you authorship credit | 14:20 |
| JayF | I've done that in the past before for people outside of the openstack community who had one or two patches they wanted to get in | 14:20 |
| GhostLyr_ | hahaha, thank you a lot. I don't care at all about who it's attributed to as long as I can remove local workarounds xD | 14:21 |
| JayF | Yeah, and sorry that bug got missed - we are all extremely busy. If you ever have something pressing, irc is often the way to get it addressed quickly. | 14:22 |
| GhostLyr_ | (not your issue) I first needed to find an IRC client for macOS that still works in 2024 | 14:23 |
| GhostLyr_ | looks like tkajinam just browsed through all open issues quickly. Thanks for that. | 14:24 |
| tkajinam | JayF, leave it for me. Started looking into the unit tests we may have to add | 14:33 |
| tkajinam | having code fix is not always enough . We need appropriate unit test coverage which is usually more painful | 14:33 |
| JayF | Sounds good, thanks | 14:34 |
| JayF | GhostLyr_: fwiw, the irccloud.com free tier is pretty much ideal for people who don't use IRC often | 14:35 |
| opendevreview | Merged openstack/oslo.messaging master: Display the reply queue's name in timeout logs https://review.opendev.org/c/openstack/oslo.messaging/+/754049 | 14:39 |
| opendevreview | Takashi Kajinami proposed openstack/etcd3gw master: Apply timeout in each request call https://review.opendev.org/c/openstack/etcd3gw/+/908585 | 14:42 |
| tkajinam | seems there is also https://webchat.oftc.net/ ? | 14:43 |
| tkajinam | though a tricky point is that you have to login to auth server to join OpenStack channels | 14:43 |
| tkajinam | Afaik it was made mandatory due to bots | 14:43 |
| JayF | Yeah, IRCCloud free tier also gives a pseudo-persistent presence (even though it only keeps you logged in for 4 hours idle, I believe it will show you backlog as long as other irccloud users were in the channel and/or it's on an IRCv3 network) | 14:44 |
| GhostLyr_ | tkajinam: I saw that my workplace solved the timeout slightly differently internally. instead of an extra argument we stuff it into kwargs. `kwargs["timeout"] = self.session.timeout` | 14:45 |
| GhostLyr_ | I don't know which is better or cleaner :shrug: | 14:45 |
| tkajinam | it works exactly in the same way iiuc | 14:45 |
| tkajinam | if you do it in that base _request method | 14:46 |
| GhostLyr_ | that second unit test doesn't look right to me. If you have a timeout, you can't have a status code. | 14:47 |
| GhostLyr_ | unless it was retried or another server answered | 14:48 |
| tkajinam | GhostLyr_, it does not reproduce the timeout, but ensures timeout is passed | 14:50 |
| tkajinam | I mean, it ensures the request is called with timeout set | 14:50 |
| tkajinam | I can add a case to check how status behaves when timeout is actually raised but that's not much as important as asserting args in request method call. | 14:52 |
| tkajinam | how Etcd3Client.status() behaves, I mean | 14:52 |
| opendevreview | Takashi Kajinami proposed openstack/etcd3gw master: Apply timeout in each request call https://review.opendev.org/c/openstack/etcd3gw/+/908585 | 14:54 |
| tkajinam | fixed pep8 error. need to wait until functional jobs pass | 14:55 |
| GhostLyr_ | true, asserting it has been raised or having something raise would be necessary if the etcd3gw lib handled the exception, but for now just bubbling the requests exception upwards works fine for me | 14:58 |
| GhostLyr_ | thank you so much for your effort :) | 14:58 |
| tkajinam | GhostLyr_, Ideally we need a functional tests to test its behavior with actual timeout but I'm not too sure I can add it immediately | 14:59 |
| tkajinam | so I'd move it forward with current minimum test coverage. | 15:00 |
| GhostLyr_ | can you give me a few more minutes to try and whip up that test for you? | 15:01 |
| GhostLyr_ | how do I find the git branch for you just sent? It doesn't seem to b bug/2012261 | 15:06 |
| GhostLyr_ | found it. | 15:06 |
| tkajinam | I know you found it, but the easiest way is to click the three dots at the right top corner of gerrit UI | 15:10 |
| tkajinam | and then click "Download Patch" | 15:10 |
| tkajinam | it gives you commands to download that change | 15:11 |
| tkajinam | if you have git-review installed then git review -d <number> downloads the change to your local working directory | 15:11 |
| tkajinam | like | 15:11 |
| tkajinam | git review -d 908585 | 15:11 |
| tkajinam | or | 15:11 |
| tkajinam | git review -x 908585 (to fetch the patch) | 15:11 |
| tkajinam | s/fetch/cherry-pick/ | 15:12 |
| tkajinam | GhostLyr_, ^^^ | 15:12 |
| GhostLyr_ | tkajinam: can you try this? https://gist.github.com/GhostLyrics/c5fc70ff002101681174d36f628f94a1 I'm not familiar with asserting for raised exceptions without relying on pytest, so I'm not sure I called assertRaises on the correct object :/ | 15:21 |
| GhostLyr_ | okay, got the tests to run, but mine needs more work | 15:30 |
| opendevreview | Merged openstack/oslo.reports master: Add missing direct imports https://review.opendev.org/c/openstack/oslo.reports/+/907648 | 15:31 |
| opendevreview | Takashi Kajinami proposed openstack/etcd3gw master: Apply timeout in each request call https://review.opendev.org/c/openstack/etcd3gw/+/908585 | 16:06 |
| tkajinam | GhostLyr_, ^^^ See this | 16:06 |
| tkajinam | It's 1 am here so expect no update for some time. | 16:07 |
| opendevreview | Takashi Kajinami proposed openstack/etcd3gw master: Apply timeout in each request call https://review.opendev.org/c/openstack/etcd3gw/+/908585 | 16:08 |
| GhostLyr_ | tkajinam: no worries. It's similar to what I just arrived myself: https://gist.github.com/GhostLyrics/c5fc70ff002101681174d36f628f94a1#file-test_client-py-L201 | 16:20 |
| GhostLyr_ | I struggled because I kept patching the wrong object | 16:21 |
| GhostLyr_ | wasn't aware you could attach side effects like attributes | 16:22 |
Generated by irclog2html.py 2.17.3 by Marius Gedminas - find it at https://mg.pov.lt/irclog2html/!