Hello,

I've created a new package for InfluxDB::HTTP as the
libinfluxdb-http-perl package and I don't have DM or DD access. I think
that it should be mostly ready for review.

https://salsa.debian.org/perl-team/modules/packages/libinfluxdb-http-perl

Can I gently ask for some help with reviewing my work on this package?


One detail that I left out: there's a t/ directory with one file that
contains tests. I've tried to add build-deps for running the test suite
but I ended up needing another library, Object::Result, which is not
packaged in Debian and I didn't want to fall into a rabbit hole. So for
now the package does not run tests with autopkgtest.

Cheers!

--- SoupGate-Win32 v1.05
* Origin: fsxNet Usenet Gateway (21:1/5)

On Sun, 19 Jun 2022 16:13:00 -0400, Gabriel Filion wrote:

I've created a new package for InfluxDB::HTTP as the libinfluxdb-http-perl package and I don't have DM or DD access. I think that it should be mostly ready for review. https://salsa.debian.org/perl-team/modules/packages/libinfluxdb-http-perl

Nice to see your first package coming along :)

Can I gently ask for some help with reviewing my work on this package?

Some notes:
- I wouldn't install the README.pod as it is the same as the POD (and
the manpage)
% diff -u <(pod2text README.pod) <(pod2text lib/InfluxDB/HTTP.pm)
`perldoc InfluxDB::HTTP' and `man InfluxDB::HTTP' should be enough
:)
- W: libinfluxdb-http-perl source: no-nmu-in-changelog
W: libinfluxdb-http-perl source: source-nmu-has-incorrect-version-number 0.04-1
That's because you use a different mail adress in d/control and
d/changelog.
- I: libinfluxdb-http-perl: synopsis-is-a-sentence "Perl way to interact with InfluxDB."
--> remove full stop at the end
- I: libinfluxdb-http-perl: typo-in-manual-page usr/share/man/man3/InfluxDB::HTTP.3pm.gz line 151 reponse response
Upstream might be happy about a patch :)

One detail that I left out: there's a t/ directory with one file that contains tests. I've tried to add build-deps for running the test suite but
I ended up needing another library, Object::Result, which is not packaged in Debian and I didn't want to fall into a rabbit hole. So for now the package does not run tests with autopkgtest.

Now that is interesting; typically we have t/*.t or the old test.pl,
but t/test.pl is something, hm, unusual.
And it's not run during build (and would probably need a running
influxd …)

So you can also remove the libtest-simple-perl build dependency
(coming from META.*; not that this changes anything, as it's provided
by src:perl).

As for autopkgtests, I'd still add the
"Testsuite: autopkgtest-pkg-perl"
field, as there are more tests than running the smoke tests.

And they work. "Work" not in "pass" but in the sense of "find an issue" :)

# Can't locate JSON/MaybeXS.pm in @INC (you may need to install the JSON::MaybeXS module) (@INC contains: /etc/perl /usr/local/lib/x86_64-linux-gnu/perl/5.34.0 /usr/local/share/perl/5.34.0 /usr/lib/x86_64-linux-gnu/perl5/5.34 /usr/share/perl5 /usr/lib/
x86_64-linux-gnu/perl-base /usr/lib/x86_64-linux-gnu/perl/5.34 /usr/share/perl/5.34 /usr/local/lib/site_perl) at /usr/share/perl5/InfluxDB/HTTP.pm line 13.

So libjson-maybexs-perl is missing in Depends (and the upstream
metadata).

Next one:
# Can't locate LWP/UserAgent.pm in @INC (you may need to install the LWP::UserAgent module) (@INC contains: /etc/perl /usr/local/lib/x86_64-linux-gnu/perl/5.34.0 /usr/local/share/perl/5.34.0 /usr/lib/x86_64-linux-gnu/perl5/5.34 /usr/share/perl5 /usr/lib/
x86_64-linux-gnu/perl-base /usr/lib/x86_64-linux-gnu/perl/5.34 /usr/share/perl/5.34 /usr/local/lib/site_perl) at /usr/share/perl5/InfluxDB/HTTP.pm line 14.

So let's add libwww-perl to Depends.

And let's look at the actual code:

use JSON::MaybeXS;
use LWP::UserAgent;
use Object::Result;
use URI;

We have liburi-perl but *drumroll* we're back to packaging Object::Result
which you wanted to avoid :)

I added a note to d/changelog.


Some more details:
* I've added a debian/upstream/metadata file (dh-make-perl didn't find
the github repo but lintian-brush did).
* d/control:
Maintainer: Debian Perl Group <pkg-perl-maintainers@lists.alioth.debian.org>
Uploader: Gabriel …
Vcs-Browser: https://salsa.debian.org/perl-team/modules/packages/libinfluxdb-http-perl
Vcs-Git: https://salsa.debian.org/perl-team/modules/packages/libinfluxdb-http-perl.git
(dh-make-perl has a --pkg-perl option)


Cheers,
gregor

--
.''`. https://info.comodo.priv.at -- Debian Developer https://www.debian.org
: :' : OpenPGP fingerprint D1E1 316E 93A7 60A8 104D 85FA BB3A 6801 8649 AA06
`. `' Member VIBE!AT & SPI Inc. -- Supporter Free Software Foundation Europe
`-

-----BEGIN PGP SIGNATURE-----

iQKTBAEBCgB9FiEE0eExbpOnYKgQTYX6uzpoAYZJqgYFAmKw4NpfFIAAAAAALgAo aXNzdWVyLWZwckBub3RhdGlvbnMub3BlbnBncC5maWZ0aGhvcnNlbWFuLm5ldEQx RTEzMTZFOTNBNzYwQTgxMDREODVGQUJCM0E2ODAxODY0OUFBMDYACgkQuzpoAYZJ qgZowA//U+eMh0V4W7rYaL62mLs42M7n4XuYJyYuXPIMzeBvSSYebshemn2Cubng 2fdxlr8rHp/263KTh+igepMaYqjUU6SSejDKdRlhM/pKj9ycb40ePQVejzV8yOiq oC487pPY0fb+zmN9Zy4H1L3xvwMj8ErzRLxdZALT+tr+HSH6bh6y5Ie5tdVZX17o iEYtXed5+XlhUc0I6QndBEfbTUxhjfY4ZVvHHynByFC5aQt55bhRY0GrC4IO5wMN 1rwOxlVJ3BHD7hkGHn1W/wscUvdS45Xirgipe+amKcl06PFXeQ9pV1ZUBqZFOdAZ 6xy1gRb94ta3DsoWZOHzysq9CF06Loak9Qz/WA4h8OHgWwlksbQbZojWjS6DQkCp P7c7Um+zF5KQ+mixOJQqgk45nf87/0JkJY92nHYPmuagJKvh7hXtO4HK9c8Iwtmn EgNdmOBZmGK7lTyiZ2PX3nVCDHzVF4ytcxN7EkYVEyhHZCBLC403Zk3uXjlqQikE
01S2yQ9S

Hi,

I'm finally reaching out to this small project

Thanks again Gregor for the insightful feedback you sent me back in june!

I've sent commits to implement the changes that were suggested:

On 2022-06-20 17:04, gregor herrmann wrote:

On Sun, 19 Jun 2022 16:13:00 -0400, Gabriel Filion wrote:

https://salsa.debian.org/perl-team/modules/packages/libinfluxdb-http-perl

Some notes:
- I wouldn't install the README.pod as it is the same as the POD (and
the manpage)
% diff -u <(pod2text README.pod) <(pod2text lib/InfluxDB/HTTP.pm)
`perldoc InfluxDB::HTTP' and `man InfluxDB::HTTP' should be enough
:)

right, duplication is not great. done!

- W: libinfluxdb-http-perl source: no-nmu-in-changelog
W: libinfluxdb-http-perl source: source-nmu-has-incorrect-version-number 0.04-1
That's because you use a different mail adress in d/control and
d/changelog.

oh, fixed. I'm currently changing jobs anyway so I should use another
email address if I want to continue maintaining the package with the team.

- I: libinfluxdb-http-perl: synopsis-is-a-sentence "Perl way to interact with InfluxDB."
--> remove full stop at the end

done

- I: libinfluxdb-http-perl: typo-in-manual-page usr/share/man/man3/InfluxDB::HTTP.3pm.gz line 151 reponse response
Upstream might be happy about a patch :)

I've sent a PR upstream to fix this typo.

So you can also remove the libtest-simple-perl build dependency
(coming from META.*; not that this changes anything, as it's provided
by src:perl).

ok, the dependency is removed.

As for autopkgtests, I'd still add the
"Testsuite: autopkgtest-pkg-perl"
field, as there are more tests than running the smoke tests.

ah yes ok, so this will run some basic perl smoke tests I guess?

I've added this field to the control file.

# Can't locate JSON/MaybeXS.pm in @INC (you may need to install the JSON::MaybeXS module) (@INC contains: /etc/perl /usr/local/lib/x86_64-linux-gnu/perl/5.34.0 /usr/local/share/perl/5.34.0 /usr/lib/x86_64-linux-gnu/perl5/5.34 /usr/share/perl5 /usr/lib/

x86_64-linux-gnu/perl-base /usr/lib/x86_64-linux-gnu/perl/5.34 /usr/share/perl/5.34 /usr/local/lib/site_perl) at /usr/share/perl5/InfluxDB/HTTP.pm line 13.

So libjson-maybexs-perl is missing in Depends (and the upstream
metadata).

Next one:
# Can't locate LWP/UserAgent.pm in @INC (you may need to install the LWP::UserAgent module) (@INC contains: /etc/perl /usr/local/lib/x86_64-linux-gnu/perl/5.34.0 /usr/local/share/perl/5.34.0 /usr/lib/x86_64-linux-gnu/perl5/5.34 /usr/share/perl5 /usr/

lib/x86_64-linux-gnu/perl-base /usr/lib/x86_64-linux-gnu/perl/5.34 /usr/share/perl/5.34 /usr/local/lib/site_perl) at /usr/share/perl5/InfluxDB/HTTP.pm line 14.

So let's add libwww-perl to Depends.

added both

And let's look at the actual code:

use JSON::MaybeXS;
use LWP::UserAgent;
use Object::Result;
use URI;

We have liburi-perl but *drumroll* we're back to packaging Object::Result which you wanted to avoid :)

oh blah you're totally right. the Object::Result is used at runtime in
the library, so there's no escaping packaging it!

I took a quick look at the code from Object::Result and all dependencies
are already packaged in debian.

I'll try and create another new package for that second lib soon.

--- SoupGate-Win32 v1.05
* Origin: fsxNet Usenet Gateway (21:1/5)

On Tue, 16 Aug 2022 02:40:37 -0400, Gabriel Filion wrote:

I'm finally reaching out to this small project

Yay :)

Thanks again Gregor for the insightful feedback you sent me back in june!

You're welcome.

- I: libinfluxdb-http-perl: typo-in-manual-page usr/share/man/man3/InfluxDB::HTTP.3pm.gz line 151 reponse response
Upstream might be happy about a patch :)

I've sent a PR upstream to fix this typo.

Great.

So you can also remove the libtest-simple-perl build dependency
(coming from META.*; not that this changes anything, as it's provided
by src:perl).

ok, the dependency is removed.

(Looking at the commit message: It's more that it's not used than
that Test::More is also in src:perl; the latter just means that the
separate libtest-simple-perl wouldn't have been installed as it's
also Provided.)

As for autopkgtests, I'd still add the
"Testsuite: autopkgtest-pkg-perl"
field, as there are more tests than running the smoke tests.

ah yes ok, so this will run some basic perl smoke tests I guess?

See https://perl-team.pages.debian.net/autopkgtest.html for a
description of the tests.
(In this unusual case smoke.t is not run, as there is no t/*.t and no
test.pl, but both use.t and syntax.t caught the missing
dependencies.)

# Can't locate JSON/MaybeXS.pm in @INC (you may need to install the JSON::MaybeXS module) (@INC contains: /etc/perl /usr/local/lib/x86_64-linux-gnu/perl/5.34.0 /usr/local/share/perl/5.34.0 /usr/lib/x86_64-linux-gnu/perl5/5.34 /usr/share/perl5 /usr/

lib/x86_64-linux-gnu/perl-base /usr/lib/x86_64-linux-gnu/perl/5.34 /usr/share/perl/5.34 /usr/local/lib/site_perl) at /usr/share/perl5/InfluxDB/HTTP.pm line 13.

# Can't locate LWP/UserAgent.pm in @INC (you may need to install the LWP::UserAgent module) (@INC contains: /etc/perl /usr/local/lib/x86_64-linux-gnu/perl/5.34.0 /usr/local/share/perl/5.34.0 /usr/lib/x86_64-linux-gnu/perl5/5.34 /usr/share/perl5 /usr/

lib/x86_64-linux-gnu/perl-base /usr/lib/x86_64-linux-gnu/perl/5.34 /usr/share/perl/5.34 /usr/local/lib/site_perl) at /usr/share/perl5/InfluxDB/HTTP.pm line 14.

added both

Cool.

And let's look at the actual code:

use JSON::MaybeXS;
use LWP::UserAgent;
use Object::Result;
use URI;

We have liburi-perl but *drumroll* we're back to packaging Object::Result which you wanted to avoid :)

liburi-perl is still missing in Depends.
Well, and the not-yet-packaged libobject-result-perl as well.

oh blah you're totally right. the Object::Result is used at runtime in the library, so there's no escaping packaging it!

Ack.

I took a quick look at the code from Object::Result and all dependencies are already packaged in debian.
I'll try and create another new package for that second lib soon.

Just shout when it's in git :)


Cheers,
gregor

--
.''`. https://info.comodo.priv.at -- Debian Developer https://www.debian.org
: :' : OpenPGP fingerprint D1E1 316E 93A7 60A8 104D 85FA BB3A 6801 8649 AA06
`. `' Member VIBE!AT & SPI Inc. -- Supporter Free Software Foundation Europe
`-

-----BEGIN PGP SIGNATURE-----

iQKTBAEBCgB9FiEE0eExbpOnYKgQTYX6uzpoAYZJqgYFAmL7XXFfFIAAAAAALgAo aXNzdWVyLWZwckBub3RhdGlvbnMub3BlbnBncC5maWZ0aGhvcnNlbWFuLm5ldEQx RTEzMTZFOTNBNzYwQTgxMDREODVGQUJCM0E2ODAxODY0OUFBMDYACgkQuzpoAYZJ qgYAnBAAkhNNYRLPdRMq4lDW55vlPrBmsa0Kqtm0JBSmx0zioLh9ZlN7C3rCjyRF dBXdt0aEdr5KANZ14wdmZHBiBmD1dyn4UrfITVEgU0RxGkyboK0B4mD0Pfl39cOu PrUz8LRQ9M1dOJ1L+gpdGDZ3mRu12i9WgV+56XfTvVhZhg+9LB+QwmG7hlA6DOFQ 6P8F1to0OEfTk1PVEkoAYExyZVN8c+ghr00oBKUK3t+GHg2Rf0NQEa52j8BfNgej QGUmWgDZqdbMzb4HhjIK/+TObROpFQvZsVVmQvmUYWUeNjUpSKHvuW5jMrGe6Us6 2hpOCpRX5qR86yM8ecLCp6M+kq8afjuusmLFlTmgs2ZTVTUo909L7xqthnuiX2TZ Vz8d4ZwChkCW31W4td/NmP9spHavjH0cm3k3hltH8dN5f72g2fzlTRG5o6r1+TQf +6Y7IsyNvXlkVX6sFnxQhEm1PjDNlTJLDNUHRqUQ1BwTeApfxqFAKqpzuyRGabNq sW+J5E8ReMdJOtmYFrrbHqCmDKw6OIRL2acbLtuwAFOF52pt4WzsHrc866mLdqQC 6qs+O9lPv2orlYqraI9ni9LA7Qwc8MPyoTtusY5y+yqPE6PThup+/3kqDw1WcAlJ sOP4GTBOMkwY5z4fJesV+1WCGlpSNwyG7UCWOZ8+YV7Dj+nRl64=
=O5N3
-----END PGP SIGNATURE-----

--- SoupGate-Win32 v1.05
* Origin: fsxNet Usenet Gateway (21:1/5)

Hello again Perl Team,

I'm finally coming back to this during a local meetup. From what I could
see there are mostly two things that are still needed:
1) missing dependency -- that's now in debian!
2) configuring autopkgtest. that seems more difficult (more below)

On 2022-08-16 05:03, gregor herrmann wrote:

And let's look at the actual code:

use JSON::MaybeXS;
use LWP::UserAgent;
use Object::Result;
use URI;

We have liburi-perl but*drumroll* we're back to packaging Object::Result >>> which you wanted to avoid 🙂

liburi-perl is still missing in Depends.
Well, and the not-yet-packaged libobject-result-perl as well.

oh blah you're totally right. the Object::Result is used at runtime in the >> library, so there's no escaping packaging it!

Ack.

I took a quick look at the code from Object::Result and all dependencies are >> already packaged in debian.
I'll try and create another new package for that second lib soon.

Just shout when it's in git 🙂

libobject-result-perl is now in the debian archive.
Both liburi-perl and libobject-result-perl are marked as dependencies in
the control file.


I have a local branch where I started to look at enabling autopkgtest,
which mostly just adds test-only dependencies to the control file..
However, when reading the source code of the file t/tests.pl I can see
that it's expecting to contact a running InfluxDB instance. There is no
perl library for testing influxdb (e.g. something like Test::InfluxDB)
that I can see that would make this possible to rewire.
I honestly don't know how to get InfluxDB running.. I'm only packaging libinfluxdb-http-perl because it's a new dependency of the smokeping
package.

So I'm wondering if getting libinfluxdb-http-perl into the archive
without autopkgtest would be an option. If I can get some help it would
still be possible to enable those tests later.


cheers

--- SoupGate-Win32 v1.05
* Origin: fsxNet Usenet Gateway (21:1/5)

On Sun, 01 Oct 2023 14:45:08 -0400, Gabriel Filion wrote:

I have a local branch where I started to look at enabling autopkgtest, which mostly just adds test-only dependencies to the control file.. However, when reading the source code of the file t/tests.pl I can see that it's expecting to contact a running InfluxDB instance.

Some random thoughts and observations:

- autopkgtest-pkg-perl typically runs three tests; even if we can't run
the smoke test (but more on that below), having the others enabled
would be nice
- I noticed that the tests are also not run during build (as `make
test' won't run t/test.pl) which is also not so nice
- reading t/test.pl it looks like an influxdbd will be started by
the test script …

After playing a bit getting them to run during build was not
difficult in th end :)

Now for the autopkgtest's smoke test …
*scratches head*
*tries something with smoke-setup*

Oh, this works :)
(rename t/test.pl to test.pl, then it gets picked up)

Alright, pushed to an "enabletests" branch.


Side note: renaming ./t/test.pl to ./test.pl (upstream or with a
quilt patch) would probably make both the manual invocation in
d/rules and the smoke-setup trick obsolet. But they are also neither
hard to understand nor to maintain.

So I'm wondering if getting libinfluxdb-http-perl into the archive without autopkgtest would be an option. If I can get some help it would still be possible to enable those tests later.

With my commits it seems to work, but if there are any concerns about
starting an influxdb instance during build or autopkgtests we can
also skip it. But maybe let's try and see first :)


Cheers,
gregor

--
.''`. https://info.comodo.priv.at -- Debian Developer https://www.debian.org
: :' : OpenPGP fingerprint D1E1 316E 93A7 60A8 104D 85FA BB3A 6801 8649 AA06
`. `' Member VIBE!AT & SPI Inc. -- Supporter Free Software Foundation Europe
`-

-----BEGIN PGP SIGNATURE-----

iQKTBAEBCgB9FiEE0eExbpOnYKgQTYX6uzpoAYZJqgYFAmUZyYdfFIAAAAAALgAo aXNzdWVyLWZwckBub3RhdGlvbnMub3BlbnBncC5maWZ0aGhvcnNlbWFuLm5ldEQx RTEzMTZFOTNBNzYwQTgxMDREODVGQUJCM0E2ODAxODY0OUFBMDYACgkQuzpoAYZJ qgb1NQ/+Kj72O7YbeCxyhPrJwuW1Cvc9ST1ujw0LHEikLb/73JS/cOLd1s9Vo+lv bk0YeEwc/a49Nu9e8/aiN+vnrH/R+TGxEkcmHJjPJECXQ79d3X6fo0jqZvvizbCb HNTO0wkq+IHblubl733ZDnQsZhnKstECeoGiw29KVH9fUKElFLfhKhqUGyaTOhM2 dRa6wEQHegCWCnaXYrucaiFMEjhxaKRXOH19rw1mjTdx17F74mCjYaMbK8z1JKaB 5DDcynNymDkKpgpN/O0c+YqNgrVN4czOhbx+AiGmtS3G+feSHaBgR6S4pALkY44P EdCLJhhBmXtUvro+9Vtnf3dACu2uy9dOja6iRU1uisgLvlQviR9GTgd73W5mUmFH BwELpKOC2TvQP9mrzIyHVBtCiAJ03iE7fzFDDnkfQ6z6igAYkiKrsVwMqzBU0WHZ zLEaXawResdvN7eHSO1cOOgbEoK8ZzIUJjhqtd8fFxNp9S9a4HBdwkkwsYgTrHEK
HoW3Lqtf

On Sun, 19 Nov 2023 16:33:24 -0500, Gabriel Filion wrote:

So I've added a patch that changes the rpc port in the test file and now I get a fully running build + autopkgtest process. I've also forwarded the patch upstream since I think it adds a bit of stability in their test suite.

Oh, cool.

So, I think that I am ready to request again some help with reviewing the work for the package libinfluxdb-http-perl and then if all seems in order, for sponsoring the push to NEW.

Right; reviewed, made 2 super-tiny changes, and uploaded to NEW.

(PS: For the future: Please set the distribution to unstable in
d/changelog if you think a package is ready for helpful. Helps both
reviewers to avoid building twice :) and also dpt-ready-for-upload;
e.g. with `dch -r')

Cheers,
gregor

--
.''`. https://info.comodo.priv.at -- Debian Developer https://www.debian.org
: :' : OpenPGP fingerprint D1E1 316E 93A7 60A8 104D 85FA BB3A 6801 8649 AA06
`. `' Member VIBE!AT & SPI Inc. -- Supporter Free Software Foundation Europe
`-

-----BEGIN PGP SIGNATURE-----

iQKTBAEBCgB9FiEE0eExbpOnYKgQTYX6uzpoAYZJqgYFAmVeRhtfFIAAAAAALgAo aXNzdWVyLWZwckBub3RhdGlvbnMub3BlbnBncC5maWZ0aGhvcnNlbWFuLm5ldEQx RTEzMTZFOTNBNzYwQTgxMDREODVGQUJCM0E2ODAxODY0OUFBMDYACgkQuzpoAYZJ qgbu3g//at9si9ySk1C4vMme4p2GQc+AD+cu8tIKbdd7ZSsLWWOIU+BQA6rEoLcj HjLv2Q8kD7UGKQTNVTKt2lIpdGMEMVfkyltjxRvEbwlIvC8Mz8OPRbsaVMpu0Oll rnACtBTfcZZJZ2Rj6UwSCpg2BLgwahXqFkPm7kDsIlw5ypC9DmqY5E5wHvR2Afc+ ozsFjLPsA78G8/PCJXjeMv7vbFYq+wnwcQF1P19zRNzZk0wjkZ0Hg9WjkEhjV59H eu86XLajImh1Vjdrq4/UXZi3gjv+H65C7r+KwwYllefk+Pe11j9b0keIFSdCLMcH tKxesHkR+yaISxSAQvzxwCb1Rqi/h5nD8D33llR4LTzQn3J+XcI8RYyzu//IyjqC F04JJrHuLIMKf8RbaQim6MzCT/Y3kURY3a6Av2Sm+F13BdSuVe0kYkzY3dhIUMkw gmT6RZPIymnSdoODWjbbhRU6Bg6ORNNRttSRH6cyIFTYqmPWNAAVg8ViqHVeyzEH OyqqOVRbSsZJI3EFCpyrO/ApvaLXNb2mxjezvrQPU0GYBrB6Imbi/pA2m2fhhJrL lqHDsOaXcHgrF2mKNIKp9MboqD69NaSHbgBo4kwjXhGtL7w75uxqJoP97n+TlWJU RJ2x+/BqYwa4M+kYKODfjTNWXMlyIv9vHek3++WUDFcgyEuqAu0=
=fD21
-----END PGP SIGNATURE-----

--- SoupGate-Win32 v1.05
* Origin: fsxNet Usenet Gateway (21:1/5)