pdreker / fritz_exporter

Prometheus exporter for Fritz!Box home routers
Other
144 stars 32 forks source link

2.3.0 packaged, tests fine, doesn't start #265

Closed 0-wiz-0 closed 6 months ago

0-wiz-0 commented 6 months ago

I've packaged 2.3.0 for pkgsrc, and it passes the self tests:

tests/test_config.py ..........                                                                                                                 [ 16%]
tests/test_datadonation.py ...............                                                                                                      [ 41%]
tests/test_fritzdevice.py ..........................                                                                                            [ 85%]
tests/test_main.py .........                                                                                                                    [100%]

--------- coverage: platform netbsd10, python 3.11.7-final-0 ---------
Coverage XML written to file coverage.xml

================================================================= 60 passed in 0.55s ==================================================================

However, when I try starting fritzexporter, I just get an error:

# fritzexporter-3.11 
Traceback (most recent call last):
  File "/usr/pkg/bin/fritzexporter-3.11", line 8, in <module>
    sys.exit(__main__())
             ^^^^^^^^^^
TypeError: 'module' object is not callable
# cat /usr/pkg/bin/fritzexporter-3.11               
#!/usr/pkg/bin/python3.11
# -*- coding: utf-8 -*-
import re
import sys
from fritzexporter import __main__
if __name__ == "__main__":
    sys.argv[0] = re.sub(r"(-script\.pyw|\.exe)?$", "", sys.argv[0])
    sys.exit(__main__())

I don't know what this means, do you know?

I also tried the other way that is mentioned on readthedocs:

python -m fritz_export_helper <FRITZ-IP> <USERNAME> <PASSWORD> <ServiceName> <ActionName> # Will output the data returned from the device in a readable format

but this gives

# python -m fritz_export_helper
/usr/pkg/bin/python3.11: No module named fritz_export_helper

and indeed, fritzexporter only installs a 'fritzexporter' module.

pdreker commented 6 months ago

Please see the very end of this documentation (ignoring the 'only run this in docker' part for netbsd).

When looking at the code for your fritzexporter-3.11 script (Where does that come from?) it imports the special name __main__ from the module fritzexporter and the tries calling it via __main__(). This just seems... "off" to me, but I may be missing something. Normally __main__ is simply the name of the environment of the "top-level code" (https://docs.python.org/3/library/__main__.html), so I would never expect this to be "callable".

The fritzexporter module is not really intended to be used via an import (which is what the script tries to do).

fritz_exporter_helper in turn is just another script (see this repo, at the top level), which is used for something else entirely, so that will not help.

If you want to call the exporter directly from a shell I would suggest using a bash/sh script which just does python-3.11 -m fritzexporter <PARAMETERS> with some error checking.

0-wiz-0 commented 6 months ago

python-3.11 -m fritzexporter <PARAMETERS> does indeed work, thanks.

The script comes from your pyproject.toml file, I think.

pyproject.toml has:

[tool.poetry.scripts]
fritzexporter = 'fritzexporter:__main__'

(The "-3.11" comes from pkgsrc because we try to make different Python versions of the same package parallel-installable.)

Will you adapt pyproject.toml so this file is not created, or so it's useful?

More details about how this is built: we use the build module, since that is as I understand it current best practice for building Python modules from source, and then install this using the installer module (also best practice AFAICT):

I.e. python -m build --wheel --some-other-args to get a wheel, then install it using python -m installer and package that up into our native format.

The wheel does not contain the fritzexporter script, but it has a fritz_exporter-2.3.0.dist-info/entry_points.txt file, which is:

[console_scripts]
fritzexporter=fritzexporter:__main__

and installer then creates the script, AFAICT.

pdreker commented 6 months ago

You never stop learning things... :-)

The line in pyproject.toml is quite obviously wrong, but I think it comes from the boilerplate from poetry init. Not sure, but either way, it is wrong.

The problem is, that the package was never intended to be run from an import statement and as such never tested. Also it does not have a clean entrypoint - I tried importing the main() function from fritzexporter.__main__.,pybut this produces a circular import.

I will look into this, but as of now I don't have a clear idea for an easy fix. Changing the pyproject.toml line will probably not be enough.

pdreker commented 6 months ago

Seems like the "scripts" line can just be removed.

Before I push this and roll a new release could you please test this? Simply remove the the two lines:

[tool.poetry.scripts]
fritzexporter = 'fritzexporter:__main__'

and try repackaging. If this works I'll push the change and release 2.3.1 which contains the fix.

0-wiz-0 commented 6 months ago

Removing these two lines works for me - the fritzexporter script is not created any longer.

pdreker commented 6 months ago

I have just released 2.3.1. The fix does not appear in the Changelog, as it is "build only" which is filtered from there.

I'll close this as completed. If this does not fix it, feel free to re-open.

Thanks for your feedback :-)