allowing for max connection configuration #939

PeterSurda · 2017-01-10T20:47:30+01:00

ghost commented

2017-01-10 20:47:30 +01:00

(Migrated from github.com)

This allows for the maximum number of connections to be configured in the settings UI.

PeterSurda commented

2017-01-10 20:59:32 +01:00

(Migrated from github.com)

Please change the variable name to maxoutboundconnections or something like that, because maxconnections is confusing. Also please change label_26 accordingly.

I'm redesigning the network subsystem to asyncore so most of it will be scrapped, but the GUI changes and the variable I plan on keeping.

Other than that it looks ok, please give me a couple of days for a closer code review, I'm fixing some other stuff so don't have time today. I don't think I'll have to test it very long.

Please change the variable name to maxoutboundconnections or something like that, because maxconnections is confusing. Also please change label_26 accordingly. I'm redesigning the network subsystem to asyncore so most of it will be scrapped, but the GUI changes and the variable I plan on keeping. Other than that it looks ok, please give me a couple of days for a closer code review, I'm fixing some other stuff so don't have time today. I don't think I'll have to test it very long.

ghost commented

2017-01-10 21:00:14 +01:00

(Migrated from github.com)

Will do. Thanks.

ghost commented

2017-01-10 21:05:24 +01:00

(Migrated from github.com)

Done.

bmng-dev (Migrated from github.com) reviewed 2017-01-11 03:58:27 +01:00

src/class_sqlThread.py

bmng-dev (Migrated from github.com) commented

2017-01-11 03:58:27 +01:00

Add a check here to ensure the option is a number if it exists

bmng-dev (Migrated from github.com) reviewed 2017-01-11 04:03:09 +01:00

src/bitmessageqt/__init__.py

						
				@ -2391,6 +2391,14 @@ class MyForm(settingsmixin.SMainWindow):

				                QMessageBox.about(self, _translate("MainWindow", "Number needed"), _translate(

				                    "MainWindow", "Your maximum download and upload rate must be numbers. Ignoring what you typed."))

bmng-dev (Migrated from github.com) commented

2017-01-11 04:03:09 +01:00

Refer to the immediately preceding options to see how to ensure the user entered a number.

bmng-dev (Migrated from github.com) requested changes 2017-01-11 04:11:49 +01:00

bmng-dev (Migrated from github.com) left a comment

Ensure the option is a number when changing it and reading from disk to avoid crashing the outgoingSynSender threads and preventing new outgoing connections if it is not a number.

PeterSurda (Migrated from github.com) reviewed 2017-01-11 21:02:28 +01:00

src/class_sqlThread.py

						
				@ -436,0 +438,4 @@

				                # Ensure we have an integer

				                int(float(BMConfigParser().get('bitmessagesettings', 'maxoutboundconnections')))

				            except:

				                logger.fatal('Your maximum outbound connections must be a number.')

PeterSurda (Migrated from github.com) commented

2017-01-11 21:02:28 +01:00

This should simply log an error and set it to 8 instead of dying.

PeterSurda (Migrated from github.com) reviewed 2017-01-11 21:10:54 +01:00

src/bitmessageqt/__init__.py

						
				@ -2392,2 +2392,4 @@

				                    "MainWindow", "Your maximum download and upload rate must be numbers. Ignoring what you typed."))

				            try:

				                # Ensure we have an integer

PeterSurda (Migrated from github.com) commented

2017-01-11 21:10:54 +01:00

Instead of this whole exception handling, try something like
lineEditMaxOutboundConnections.setValidator(QIntValidator(1, 1024, lineEditMaxOutboundConnections))
in settings.py (1 being the lower limit, 1024 the top).

For a more complex example look into newchandialog.py and addressvalidator.py, in your case you don't need a new validator class, you can use the QIntValidator as it is.

Instead of this whole exception handling, try something like `lineEditMaxOutboundConnections.setValidator(QIntValidator(1, 1024, lineEditMaxOutboundConnections))` in settings.py (1 being the lower limit, 1024 the top). For a more complex example look into newchandialog.py and addressvalidator.py, in your case you don't need a new validator class, you can use the QIntValidator as it is.

PeterSurda (Migrated from github.com) reviewed 2017-01-11 21:14:05 +01:00

src/bitmessageqt/settings.py

						
				@ -157,0 +164,4 @@

				        sizePolicy.setHeightForWidth(self.lineEditMaxOutboundConnections.sizePolicy().hasHeightForWidth())

				        self.lineEditMaxOutboundConnections.setSizePolicy(sizePolicy)

				        self.lineEditMaxOutboundConnections.setMaximumSize(QtCore.QSize(60, 16777215))

				        self.lineEditMaxOutboundConnections.setObjectName(_fromUtf8("lineEditMaxOutboundConnections"))

PeterSurda (Migrated from github.com) commented

2017-01-11 21:14:05 +01:00

how about also
self.lineEditMaxOutboundConnections.setPlaceholderText(_fromUtf8("8"))

how about also `self.lineEditMaxOutboundConnections.setPlaceholderText(_fromUtf8("8"))`

PeterSurda reviewed 2017-01-11 21:14:28 +01:00

src/bitmessageqt/__init__.py

						
				@ -2392,2 +2392,4 @@

				                    "MainWindow", "Your maximum download and upload rate must be numbers. Ignoring what you typed."))

				            try:

				                # Ensure we have an integer

PeterSurda commented

2017-01-11 21:14:27 +01:00

👍

PeterSurda (Migrated from github.com) requested changes 2017-01-13 10:13:00 +01:00

src/class_sqlThread.py

						
				@ -436,0 +436,4 @@

				        if BMConfigParser().has_option('bitmessagesettings', 'maxoutboundconnections'):

				            try:

				                # Ensure we have an integer

				                int(float(BMConfigParser().get('bitmessagesettings', 'maxoutboundconnections')))

PeterSurda (Migrated from github.com) commented

2017-01-11 21:23:02 +01:00

Maybe
if BMConfigParser().getint('bitmessagesettings', 'maxoutboundconnections'))) < 1: throw ValueException

Maybe `if BMConfigParser().getint('bitmessagesettings', 'maxoutboundconnections'))) < 1: throw ValueException `

src/class_sqlThread.py

						
				@ -436,0 +437,4 @@

				            try:

				                # Ensure we have an integer

				                int(float(BMConfigParser().get('bitmessagesettings', 'maxoutboundconnections')))

				            except:

PeterSurda (Migrated from github.com) commented

2017-01-11 21:24:15 +01:00

And this should be
except ValueException:
because you're supposed to always name the exception. I haven't always done it but in new code we should try to do that.

And this should be `except ValueException:` because you're supposed to always name the exception. I haven't always done it but in new code we should try to do that.