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

Mealie Token Authentication #45

Open
wants to merge 3 commits into
base: main
Choose a base branch
from

Conversation

fberlakovich
Copy link

  • Allow to specify a Mealie token instead of username/password
  • Refactor the loading of settings to make it explicit (importing the config module no longer triggers settings validation)
    • Tests no longer depend on a potentially missing/incorrect .env file, but are configured with a fixture

The refactoring of the settings was a side effect of testing the introduced validator. If desired, I can also split the PR into two.

@fberlakovich
Copy link
Author

Sorry, I missed a view things. I will update the PR accordingly

Felix Berlakovich added 3 commits January 2, 2025 11:20

Verified

This commit was created on GitHub.com and signed with GitHub’s verified signature.
@ephes
Copy link
Owner

ephes commented Feb 21, 2025

Did you update the PR? There's also a lot of weird changes like this create_kptncook_client what does this function do?

@fberlakovich
Copy link
Author

Yes, I did update it. The core change of this PR is in 33b7529. However, I also changed a few things so the tests would not depend on local env files and got loaded automatically whenever the settings module is included. With the change, the settings object is constructed explicitly in one place and all other places receive the settings via dependency injection. But this also means that the KptnCookClient constructor parameters can no longer be default initialized from the settings object. The create_kptncook_client function simply creates a client and takes care of passing the parameters.

Like I said in the description: I can factor out the settings part into a separate PR or drop it entirely if you don't like it. I primarly introduced it because writing robust tests was a bit of a hassle.

@fberlakovich
Copy link
Author

FYI: the updated PR removed changes that PyCharm introduced into the jupyter notebooks during execution and which I accidentally committed.

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.

None yet

2 participants