properly handle integer environment values, fixes issue with specifying ports via environment vars

[thebubbleindex]

make sure tcp and udp port for dht are int type

Reason for fix (Ubuntu 20.04 and desktop app):

1. Without fix users specify env vars, LBRY_TCP_PORT=34562 and LBRY_UDP_PORT=34562, but then when you run the daemon it complains about string comparisons.
2. With this fix people can download the desktop application, set the above environment variables and have their seeding working with VPN! PIA VPN gives you a randomly assigned port forward port, with this fix a user can now use a VPN (with ENV vars).

[eukreign]

Thanks for the PR!

The proper place to fix this is in the conf.py module, it is already doing the conversion to int there, so if the conversion is not working this is a bug and we would need a test showing it failing and then the accompanying fix:

Where conversion is already being done:

lbry-sdk/lbry/conf.py

Line 101 in 8be1c83

return int(value)

Tests that need to be updated to reproduce the problem you are experiencing:

lbry-sdk/tests/unit/test_conf.py

Lines 91 to 95 in 8be1c83

	def test_environment(self):
	c = TestConfig()
	self.assertEqual(c.test_str, 'the default')
	c.set_environment({'LBRY_TEST_STR': 'from environ'})
	self.assertEqual(c.test_str, 'from environ')

[thebubbleindex]

Might need some help, I cannot find where the environment variables are being converted into a type.

lbry-sdk/lbry/conf.py

Line 277 in 1d1f052

class EnvironmentAccess:

[eukreign]

I think you've found the problem, if you look at the ArgumentAccess and ConfigFileAccess they all have load() methods which are called in __init__ to deserialize() the arguments but the EnvironmentAccess does not have one.

[thebubbleindex]

Thanks! I updated the conf.py to a working version, adding some type parsing and loading to the EnivronmentAccess

https://github.com/thebubbleindex/lbry-sdk/blob/eee50645c2c3b0ac2203716c6c6fead3134f5151/lbry/conf.py#L277

[eukreign]

It should be very close to how ArgumentAccess works and loop over all of the pre-defined settings and look for them inside the environment: for setting in self.configuration.get_settings().

Also, this needs tests, they would go here: https://github.com/lbryio/lbry-sdk/blob/master/tests/unit/test_conf.py#L91 (look at all of the other tests in that file to get inspiration).

You are making good progress, don't hesitate to ask about how something work if you are confused, this conf code in LBRY heavily uses Python descriptors which are explained here:

https://docs.python.org/3/howto/descriptor.html
https://docs.python.org/3.7/reference/datamodel.html#implementing-descriptors
https://docs.python.org/3.7/reference/datamodel.html#invoking-descriptors

[thebubbleindex]

Cool, think I have it now. I'll try to add the tests (I'm new to Python, but the examples you mentioned should be enough). Cheers!

[eukreign]

Looking good! Instead of adding two new config options to the test config could you just re-use test_int that's already there (it's type Integer).

[tzarebczan]

@thebubbleindex thanks for the PR, congrats on your first one!

Can we show you some appreciation for the contribution?

Also, please consider signing up for our dev list at the bottom of lbry.tech!