Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Allow default connect/read timeout and more to be customizable #6041

Closed
LefterisJP opened this issue May 10, 2023 · 1 comment · Fixed by #6431
Closed

Allow default connect/read timeout and more to be customizable #6041

LefterisJP opened this issue May 10, 2023 · 1 comment · Fixed by #6431
Milestone

Comments

@LefterisJP
Copy link
Member

Abstract

At the moment all connection related timeout and retries variables are hard-coded constants in the code:

QUERY_RETRY_TIMES = 5
DEFAULT_CONNECT_TIMEOUT = 15

There is no possibility to customize them depending on the user's situation and connection capabilities.

Motivation

If a user is in a slow connection place we should allow them to configure these variables.

Specification

Make them configurable settings

@LefterisJP LefterisJP added good first issue Good for newcomers easy Easy issue. But maybe not good first. Fast if you are a bit familiar. labels May 10, 2023
@LefterisJP LefterisJP modified the milestones: 1.28.0, 1.29.0 May 10, 2023
@dimyG dimyG removed the easy Easy issue. But maybe not good first. Fast if you are a bit familiar. label May 17, 2023
@dimyG
Copy link
Contributor

dimyG commented May 17, 2023

Currently the only way to access the settings is to read them from the user database with the DBHandler.get_settings method.

This creates a challenge in moving these constants to settings because these constants are used in many places, including places where the dbhandler instance is not available. For example in rotkehlchen/chain/ethereum/graph.py or rotkehlchen/icons.py and elsewhere.

We discussed two possible solutions:

  1. Pass down the dbhandler instance to all places where these constants are used (and get them from get_settings method).
  2. Cache the settings in a global object, import this object directly where you need it and read the settings you want from it.

Additional details for solution 1
The change can happen gradually, by having these values both in constants and in settings and read them from settings when you can while keep using the constants when its difficult to get access to the dbhandler.

Additional details for solution 2
We can use a singleton to cache the settings. It is initialized at user login and deleted at user logout. Here is an approach using a SettingsManager singleton class https://gist.github.com/dimyG/27322bb312ed3659370b58f9d8338261. Then you can sync_with_user_db in dbhandler get_settings method, update the cached settings in dbhandler set_settings and set_setting methods. This singleton approach though is an antipattern. We only use them for things that are really global over the entire app such as the globalDB and global prices, while the settings is specific to the user and not to the app. Many users can have account to the same app installation.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants