UPnP errors: AttributeError: Router instance has no attribute 'location' #1131

New Issue

2018-02-14T20:10:05+01:00

yurivict commented

2018-02-14 20:10:05 +01:00

(Migrated from github.com)

Getting these errors while router says "Turn UPnP On":

2018-02-14 11:06:18,080 - ERROR - Failure running UPnP router search.
Traceback (most recent call last):
  File "/usr/local/lib/python2.7/site-packages/pybitmessage/upnp.py", line 227, in run
    if router.location == newRouter.location:
AttributeError: Router instance has no attribute 'location'

I know that we worked on UPnP issues before, and UPnP worked perfectly afterwards, but that was with a different router. So this isn't necessarily a regression.

Router: Netgear WNR1000
Version: 0.6.3.2
OS: FreeBSD 11.1

Existence of such issue makes me think again that using proprietary code to do UPnP was a bad idea and you should use https://pypi.python.org/pypi/UPnP or https://pypi.python.org/pypi/miniupnpc instead.

Getting these errors while router says "Turn UPnP On": ``` 2018-02-14 11:06:18,080 - ERROR - Failure running UPnP router search. Traceback (most recent call last): File "/usr/local/lib/python2.7/site-packages/pybitmessage/upnp.py", line 227, in run if router.location == newRouter.location: AttributeError: Router instance has no attribute 'location' ``` I know that we worked on UPnP issues before, and UPnP worked perfectly afterwards, but that was with a different router. So this isn't necessarily a regression. Router: Netgear WNR1000 Version: 0.6.3.2 OS: FreeBSD 11.1 ---- Existence of such issue makes me think again that using proprietary code to do UPnP was a bad idea and you should use https://pypi.python.org/pypi/UPnP or https://pypi.python.org/pypi/miniupnpc instead.

PeterSurda commented

2018-02-15 03:01:48 +01:00

(Migrated from github.com)

The thing is, I want to move the UPnP code to asyncore as well. That may not mean I can't reuse an existing UPnP library but it's probably going to be more difficult.

yurivict commented

2018-02-15 07:01:19 +01:00

(Migrated from github.com)

I want to move the UPnP code to asyncore as well.

You want to have UPnP processed in the same event loop? IMO, this has little benefit because UPnP is a really low traffic protocol. This will be more of a problem than a solution.

> I want to move the UPnP code to asyncore as well. You want to have UPnP processed in the same event loop? IMO, this has little benefit because UPnP is a really low traffic protocol. This will be more of a problem than a solution.

PeterSurda commented

2018-02-15 10:10:47 +01:00

(Migrated from github.com)

Threads in python seem to consume a lot of memory, the purpose of this decision is to save memory.

yurivict commented

2018-02-15 10:20:24 +01:00

(Migrated from github.com)

I don't think you need threads for UPnP. UPnP actions are rare and instant. This can be done from other threads.

PeterSurda commented

2018-02-15 11:22:35 +01:00

(Migrated from github.com)

It doesn't need many resources, however some operations can block. Most of the other code I saw was synchronous so I can't use it in a thread with something else.

yurivict commented

2018-02-15 11:31:51 +01:00

(Migrated from github.com)

Just run UPnP in the separate thread as an exception. Mostly major BM threads were consuming resources. There is no need to be overly-aggressive and eliminate every thread there is, IMO. -)

There is value in reusing code. Like this issue wouldn't even have been created had UPnP been handled by the external library.

Just run UPnP in the separate thread as an exception. Mostly major BM threads were consuming resources. There is no need to be overly-aggressive and eliminate every thread there is, IMO. -) There is value in reusing code. Like this issue wouldn't even have been created had UPnP been handled by the external library.

PeterSurda commented

2018-02-15 11:33:39 +01:00

(Migrated from github.com)

Yes, that's still an option, a more urgent candidate for moving to asyncore is the DNS bootstrap.

PeterSurda commented

2018-02-18 23:19:47 +01:00

(Migrated from github.com)

I have an idea what can be done with my limited time. I'll make it so that if you have one of the modules you mentioned, it will try to use those instead of the built in. That I should be able to do rather quickly, then later I can add proper dependencies checking into setup.py and so on, but at least you'd be able to work around it simply by installing the modules.

👍 1

yurivict commented

2018-02-18 23:27:16 +01:00

(Migrated from github.com)

Ok, I will add it as RUN_DEPENDS then.

This repo is archived. You cannot comment on issues.