S3 plugin test case breaks test suite / development workflow...
Hi, I'm writing this about a specific problem and about a general problem. * The specific problem is that commit 5130c43bc1f9 ("S3 plugin: add support for accessing multiple objects", 2022-05-12) introduced a dependency on the "botocore" python module, and now "make check" fails for me, because this module is unavailable on my system.
Traceback (most recent call last): File "[...]/nbdkit/plugins/S3/nbdkit-S3-plugin", line 43, in <module> from botocore.exceptions import ClientError ModuleNotFoundError: No module named 'botocore'
Now, while the README file says, from commit 6715c3d8b3e6 ("New plugin: S3 plugin for accessing disks stored in AWS S3 and Ceph.", 2020-11-14):
For the Python plugin:
- python interpreter (version 3 only)
- python development libraries
- python unittest to run the test suite
- boto3 is required to run the S3 plugin written in Python
the "boto3" dependency had never been a "hard" one until commit 5130c43bc1f9. The language suggests that running the S3 plugin -- even for testing -- is optional. Therefore, commit 5130c43bc1f9 should have updated the test cases "test-S3.sh" and "test-S3-unit.sh" with a proper "requires" line (I assume anyway). FWIW, the following trick in "test-S3.sh":
# There is a fake boto3 module in test-S3/ which we use as a test # harness for the plugin.
Does not work. ... So, after all, the bug in commit 5130c43bc1f9 may have been that it did not add the ClientError exception type to the fake module in "tests/test-S3/boto3". I'm unsure; but please fix it anyway. * The generic problem is that I need to write this separate error report email, rather than commenting directly under the submission -- on the mailing list -- or the pull request -- on gitlab -- that ended up as commit 5130c43bc1f9. For the life of me, I just can't figure out *where* commit 5130c43bc1f9 was originally reviewed. I think that's wrong. ... Ultimately, I've found the patch in the MR listing at <https://gitlab.com/nbdkit/nbdkit/-/merge_requests?scope=all&state=all>, namely in MR#9 -- <https://gitlab.com/nbdkit/nbdkit/-/merge_requests/9>. But this merge request has status *closed*, not *merged*. So even though a commit is given, I can't find the associated discussion because (a) MR#9 is not listed in the list of *merged* merge requests (only the "rejected" ones), (b) the commit message itself does not reference MR#9. I think we should improve our process here. Thanks, Laszlo
On Tue, May 17, 2022 at 10:31:59AM +0200, Laszlo Ersek wrote:
Hi,
I'm writing this about a specific problem and about a general problem.
* The specific problem is that commit 5130c43bc1f9 ("S3 plugin: add support for accessing multiple objects", 2022-05-12) introduced a dependency on the "botocore" python module, and now "make check" fails for me, because this module is unavailable on my system.
Traceback (most recent call last): File "[...]/nbdkit/plugins/S3/nbdkit-S3-plugin", line 43, in <module> from botocore.exceptions import ClientError ModuleNotFoundError: No module named 'botocore'
Now, while the README file says, from commit 6715c3d8b3e6 ("New plugin: S3 plugin for accessing disks stored in AWS S3 and Ceph.", 2020-11-14):
For the Python plugin:
- python interpreter (version 3 only)
- python development libraries
- python unittest to run the test suite
- boto3 is required to run the S3 plugin written in Python
the "boto3" dependency had never been a "hard" one until commit 5130c43bc1f9. The language suggests that running the S3 plugin -- even for testing -- is optional.
Yeah it should definitely be optional.
Therefore, commit 5130c43bc1f9 should have updated the test cases "test-S3.sh" and "test-S3-unit.sh" with a proper "requires" line (I assume anyway).
FWIW, the following trick in "test-S3.sh":
# There is a fake boto3 module in test-S3/ which we use as a test # harness for the plugin.
Does not work.
... So, after all, the bug in commit 5130c43bc1f9 may have been that it did not add the ClientError exception type to the fake module in "tests/test-S3/boto3". I'm unsure; but please fix it anyway.
* The generic problem is that I need to write this separate error report email, rather than commenting directly under the submission -- on the mailing list -- or the pull request -- on gitlab -- that ended up as commit 5130c43bc1f9. For the life of me, I just can't figure out *where* commit 5130c43bc1f9 was originally reviewed.
It's this one: https://gitlab.com/nbdkit/nbdkit/-/merge_requests/9 It may be that you couldn't find it because I reviewed and merged the requests by hand which involved rebasing them, so gitlab didn't associate the commit with the original request.
I think that's wrong.
... Ultimately, I've found the patch in the MR listing at <https://gitlab.com/nbdkit/nbdkit/-/merge_requests?scope=all&state=all>, namely in MR#9 -- <https://gitlab.com/nbdkit/nbdkit/-/merge_requests/9>.
But this merge request has status *closed*, not *merged*. So even though a commit is given, I can't find the associated discussion because (a) MR#9 is not listed in the list of *merged* merge requests (only the "rejected" ones), (b) the commit message itself does not reference MR#9.
That's right.
I think we should improve our process here.
I should probably have added a "Fixes" link when pushing them. Rich. -- Richard Jones, Virtualization Group, Red Hat http://people.redhat.com/~rjones Read my programming and virtualization blog: http://rwmj.wordpress.com virt-p2v converts physical machines to virtual machines. Boot with a live CD or over the network (PXE) and turn machines into KVM guests. http://libguestfs.org/virt-v2v
On May 17 2022, Laszlo Ersek <lersek@redhat.com> wrote:
Hi,
I'm writing this about a specific problem and about a general problem.
* The specific problem is that commit 5130c43bc1f9 ("S3 plugin: add support for accessing multiple objects", 2022-05-12) introduced a dependency on the "botocore" python module, and now "make check" fails for me, because this module is unavailable on my system.
Apologies, I missed that. I think Richard has already fixed this since. Best, -Nikolaus -- GPG Fingerprint: ED31 791B 2C5C 1613 AF38 8B8A D113 FCAC 3C4E 599F »Time flies like an arrow, fruit flies like a Banana.«
participants (3)
-
Laszlo Ersek -
Nikolaus Rath -
Richard W.M. Jones