Refactor Paths, Logging and Routes

[mariugul]

Refactor

Paths

This was one of the most important refactors. There was extensive use of os.path which is not a great way to do paths in python, at least not anymore. I switched out most of os.path for pathlib. The reason is that instead of hardcoding paths for windows in the windows style C:\\User\\ etc. and likewise for Linux using strings /host/user/etc, pathlib already solves this issue being a cross-platform path library. It also comes with many useful methods for creating directories, files, deleting them, checking if they exist, writing, reading++.

Switching out everything for pathlib made everything easier, concise, modern, cross-platform-friendly and expandable.

Qr Code

The qr code gets saved to the tmp directory for each run of pikaraoke and gets deleted after the app stops.

Logo

The logo is included as a resource in the python package and it's fetched from the package when needed.

Logs

More Logging

Adding logger = getLogger(__name__) at the top of a file gives it a logger to use.

Configuration

I moved configuring the logging to just after the parsing of input arguments. This means the whole app can be logged.

The log config formats a easy to read log for the console and a more detailed log for the logfiles. It also gets all the loggers and formats them in the same way. Also the ones from cherrypy.

Store Logs on Host

The logging configuration now logs to a logfile with a timestamp. The path can be anywhere really. Preferably in a pikaraoke app folder somewhere. I put it in .config/pikaraoke/logs and AppData/Local/pikaraoke/Logs for Windows.

Rotate Logs

Keeps the newest 5 logfiles. Files can only grow to 10MB and will be overwritten after this on rotation. The number and size can be configured.

Separate Routes in Files

app.py was a large file with all the routes. I moved them to separate files under routes/ for more order and a cleaner app file.

Remove Global Variables

Moving the routes means there is no access to app and karaoke for example. Instead of using global variables I utilized Flask's way of registering objects and using the current_app object so we can access the karaoke object and other things in the routes no matter where they are defined.

Config File

I moved the defaults and constants from the top of app.py into a config.py. This is Flask's way of dealing with these kinds of settings. Then they are all available in the routes on current_app.config. Can move more config here later. For example tmp folder perhaps.

Parse Args in Separate File

Parsing of the input arguments for the cli has been moved out of app.py to parse_args.py. This makes it cleaner and easier to work with.

Intellisense on Args

Added a namespace to the args. This gives them intellisense. The type of some of the arguments wasn't necessarily so easy to understand. Some of them were ambiguous. I fixed the types and added this for clarity.

Docstrings and Type Hints

I added a lot of docstrings and type hints to methods. I don't think I did it everywhere. This just makes it easier to understand what's happening and to contribute to the code.

Platform Enum

Made a platform enum that holds OSX, RASPBERRY_PI, LINUX, WINDOWS and UNKNOWN so that the platform can be compared to an enum. Expanded on this and implemented methods on the enum like is_rpi() and is_windows()... so an if statement can simply be:

platform = get_platform()

if platform.is_rpi():
    pass

if platform.is_mac():
    pass

[mariugul]

@vicwomg I'm trying to merge in the last painful part of the refactor which is a bit messy. However, I have a question.

Why is it that all the endpoints seems to be accessed 3 times for every call to them? For example:

DEBUG    IP address (for QR code and splash screen): 192.168.50.152
DEBUG    IP address (for QR code and splash screen): 192.168.50.152
DEBUG    IP address (for QR code and splash screen): 192.168.50.152
INFO     Fetching available songs in: /home/pikaraoke-songs
INFO     Fetching available songs in: /home/pikaraoke-songs
INFO     Fetching available songs in: /home/pikaraoke-songs
DEBUG    self.available_songs=[]
DEBUG    self.available_songs=[]
DEBUG    self.available_songs=[]
...
DEBUG    Endpoint /splash accessed.
DEBUG    Endpoint /splash accessed.
DEBUG    Endpoint /splash accessed.
DEBUG    Endpoint /logo accessed.
DEBUG    Endpoint /logo accessed.
DEBUG    Endpoint /logo accessed.
...

[vicwomg]

@vicwomg I'm trying to merge in the last painful part of the refactor which is a bit messy. However, I have a question.

Why is it that all the endpoints seems to be accessed 3 times for every call to them?

I have never noticed this before. Either it's some kind of inherent bug or an misfire from the logger.

[mariugul]

@vicwomg I'm trying to merge in the last painful part of the refactor which is a bit messy. However, I have a question.

Why is it that all the endpoints seems to be accessed 3 times for every call to them?

I have never noticed this before. Either it's some kind of inherent bug or an misfire from the logger.

If it's not intentional it might be a bug. I think the logger reports correctly. Perhaps it's the javascript in the templates, I haven't really looked a lot into that

[vicwomg]

Looks to me like the debug and info logs are also outputting 3x so doesn’t seem specific to endpoints

…

On Sat, Sep 7, 2024 at 3:49 PM Marius C. K. ***@***.***> wrote: @vicwomg <https://github.com/vicwomg> I'm trying to merge in the last painful part of the refactor which is a bit messy. However, I have a question. Why is it that all the endpoints seems to be accessed 3 times for every call to them? I have never noticed this before. Either it's some kind of inherent bug or an misfire from the logger. If it's not intentional it might be a bug. I think the logger reports correctly. Perhaps it's the javascript in the templates, I haven't really looked a lot into that — Reply to this email directly, view it on GitHub <#396 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AA7KXNWNRIOBA6LBAU46MV3ZVN7F5AVCNFSM6AAAAABNZZG5WOVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZDGMZWGQ3TENZUGY> . You are receiving this because you were mentioned.Message ID: ***@***.***>

[mariugul]

Looks to me like the debug and info logs are also outputting 3x so doesn’t seem specific to endpoints
…
On Sat, Sep 7, 2024 at 3:49 PM Marius C. K. @.> wrote: @vicwomg https://github.com/vicwomg I'm trying to merge in the last painful part of the refactor which is a bit messy. However, I have a question. Why is it that all the endpoints seems to be accessed 3 times for every call to them? I have never noticed this before. Either it's some kind of inherent bug or an misfire from the logger. If it's not intentional it might be a bug. I think the logger reports correctly. Perhaps it's the javascript in the templates, I haven't really looked a lot into that — Reply to this email directly, view it on GitHub <#396 (comment)>, or unsubscribe https://github.com/notifications/unsubscribe-auth/AA7KXNWNRIOBA6LBAU46MV3ZVN7F5AVCNFSM6AAAAABNZZG5WOVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZDGMZWGQ3TENZUGY . You are receiving this because you were mentioned.Message ID: @.>

You are exactly right. It was the configuration of the loggers. It didn't like having logging.basicConfig() called several times. I did something like that to deal with the fact that cherrypy reconfigures it loggers in a way that is not nice, so I made an override for it. I will find a better way. Thanks.

[vicwomg]

Would it be possible to remove the logging refactor for now? Lots of good stuff otherwise in this PR

[mariugul]

Would it be possible to remove the logging refactor for now? Lots of good stuff otherwise in this PR

Yes, we can do that. I have been on vacation and just back. This PR had some issues which I now have forgotton what was. 🤓