mgeitz
commented
6 months ago This is awesome, some really great foundational refactoring changes. The use of dataclass is slick and begs the question if I should replace all these namedtuples - https://github.com/mgeitz/eqalert/blob/master/eqa/lib/struct.py#L21-L40
Note: I'll get a PR in to add on pull request triggers for cases like this to trigger workflows where no new commit in the base repository is made but for the interim I can see the passing builds in your fork
There are 3 parts to this PR. I can pull out the DataModel stuff and put it into a separate PR if you'd like (It's not actually used by anything, I just built them out and it was a lot of annoying work)
Filehanders:
Instead of calling
open()until blue in the face, instead useJSONFileHandler(and optionally dependency injection) to make file read/write calls. This makes it easier to test when it comes to functions that write/read from disk without having to monkeypatch. Instead, you can just inject your ownJSONFileHandlerthat does whatever is needed.Pathlib:
Standard library as of Python 3.4. It Objectifies paths (in a good way) - instead of working with strings you work with Path objects that have methods to do stuff like check for existence, walking trees, creating files/dirs, joining paths. And it is more OS independent out of the box than
os.pathand friends.It's a helpful library!
DataModels:
Ideally instead of keeping some arbitrary JSON/dictionary in memory to keep track of configs, there would be structured data. In this PR I added some templates for all the config types that are read/written to disk. I just used
dataclassesfrom the stdlib - however, as I think about itpydanticmight be a better choice. It's easy enough to convert to that.To check for last_state afk status you would be able do something like:
config.settings.last_state.afkinstead ofconfig["settings"]["last_state"]["afk"]. You'll be able to get help from your IDE as to what the possible objects are. And it will validate typing.This is not implemented anywhere in the code - I only set up to models to match the config file data. It's set up exactly the same, for better or worse. I found some oddities that should probably be cleaned up when actually implementing it (like
config.settings.settings.timers.spell.spell_timer_filter.filter_list.spirit_of_wolf==False). It seems like this could be better... Also, some of the JSON keys are reserved words in Python, which makes things, interesting. :)