Skip to content

AppSettings #84

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

Closed
wants to merge 12 commits into from
Closed

AppSettings #84

wants to merge 12 commits into from

Conversation

infeeeee
Copy link
Collaborator

This branch is based on #83 , that should be pulled first. It also contains changes you can see there, not everything is new here.

New feature: App Settings are stored in the configuration file, and you can edit them in the new UI. Actually I only wanted to add this to IoTuring, but the old config UI was too hard to edit, so I started with that first

Currently there are 4 settings in IoTuring/MyApp/AppSettings.py

  • Console log level: IOTURING_LOG_LEVEL still overwrites this, but it's saved in the config file
  • File log level
  • Main update interval in seconds: This is the sensor update frequency
  • Secondary update interval in minutes: This is for the future, not used anywhere yet. Sensors are updating in every couple of seconds, and the other option is a static value. Some middle ground would be useful, which updates only in a couple of minutes. E.g. DesktopEnvironment and User can change sometimes, but now they are static sensors, AppInfo shouldn't ping Github every second, that could also become a slow sensor.

Problems: Logger starts before ConfigurationIO, so Logger cannot know what is the configured level. I also wanted to add Log Folder option, but same problem. I worked it around by settings a default log level used before Configurator starts, but I don't like this solution. The app init order should be rethinked.

I was also thinking, maybe the new AppSettings as a singleton would be better? Now Settings are stored in a class variable.

Related issues: #5 #58

Other changes:

  • Update minimum Python version to 3.8, as 3.7 is EOL: https://devguide.python.org/versions/
  • importlib.metadata is available as a std lib in 3.8, so importlib_metadata backport is not needed anymore
  • Use Dockerhub links in readme and compose, instead of ghcr. It's shorter, and looks better
  • Use argparse for argument parsing. Nicer output, automatic help and version info. Try IoTuring --help

@richibrics
Copy link
Owner

richibrics commented Nov 28, 2023

Very well.
I've always postponed #5 but it was time for that option to be added.
Also I'd agree with making AppSettings as a singleton.
How is this related to #58 ?

@infeeeee
Copy link
Collaborator Author

infeeeee commented Nov 28, 2023

Main update interval in seconds: This is the sensor update frequency

You can change that interval, I called it "Main update interval in seconds"

Here in the code: https://github.com/richibrics/IoTuring/pull/84/files#diff-784cd4745c3ccfda1083a25f466edc21e4cabc7eaed3c8f7c6498a8540ae1efbR40

And than it's used by Warehouse.py here: https://github.com/richibrics/IoTuring/pull/84/files#diff-3e753f631e8c84279e975e1140329dbd4d0af8a2b6b7f7970f6862fa190a27e3R16

@richibrics
Copy link
Owner

Yes sorry I made a typo, I was asking how #58 (unique method to acquire is paths) was related to this PR

@infeeeee
Copy link
Collaborator Author

You can store that setting, I wanted to add that here as well, as I wrote:

also wanted to add Log Folder option, but same problem. I worked it around by settings a default log level used before Configurator starts, but I don't like this solution. The app init order should be rethinked.

So if the app init order is reworked, so somehow the configuration can be read before logging starts, than that option can be placed here as well.

Or if logging becomes a warehouse, than it's not related.

@infeeeee infeeeee mentioned this pull request Dec 30, 2023
5 tasks
@infeeeee
Copy link
Collaborator Author

Continued in #90

@infeeeee infeeeee closed this Dec 30, 2023
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 this pull request may close these issues.

2 participants