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

Add Household support for Mealie V2 #236

Merged
merged 16 commits into from
Aug 30, 2024

Conversation

andrew-codechimp
Copy link
Contributor

@andrew-codechimp andrew-codechimp commented Aug 24, 2024

Proposed Changes

This is a draft work in progress that I'm looking for advice/review on.

To distinguish the household support (V2) I am using a call within define_household_support to detect api availability as version checking does not work when Nightly could be V1 or V2

Most JSON responses now include a householdId which I'm having trouble with Mashumoro defining as optional, see MealPlan model for my attempt, which currently fails tests where the mealplan_today.json does not include the field, advise on how to handle optionals would be great, I've not found an example that works yet.

Also once the above is resolved advice on how to add my local lib into my local HA Core temporarily so I can test implementation there.

@joostlek
Copy link
Owner

Maybe we should create a base entity and then extend

@joostlek
Copy link
Owner

Let's ask them if we can get a better way for detecting version

@joostlek
Copy link
Owner

And maybe we should do more effort to make like a V1 submodule so we can keep supporting it

(Just also thinking out loud as I'm at F1 right now)

@andrew-codechimp
Copy link
Contributor Author

And maybe we should do more effort to make like a V1 submodule so we can keep supporting it

(Just also thinking out loud as I'm at F1 right now)

I was thinking of separate tests for V1 at least so we can continue support for a while but the code should work with either.

No problem, I don't expect quick answers to these things and won't be jumping on changing things so take your time, just putting this here to show the impact and we can iterate.

@joostlek
Copy link
Owner

Yea I think we should check with the mealie team if they have like a support window for Mealie V1, I also don't really feel like supporting that for an eternity

@andrew-codechimp
Copy link
Contributor Author

Just a heads up, the HassIO AddOn for Mealie was updated yesterday. Despite it saying v1.12.0 which is the latest prod release it has built with the V2 nightly, so the integration is now failing to setup.

I've raised an issue there to hopefully stop this in future but the damage is done and rolling back won't be an option due to DB changes.

@joostlek
Copy link
Owner

OUCH. I guess we have to make sure we fix this before the release (4 September) because 2024.8.3 is probably coming out today so we won't make that

@andrew-codechimp
Copy link
Contributor Author

andrew-codechimp commented Aug 25, 2024

Ouch indeed, expect a lot of issues being raised for the next week, of which we just got our first!
It's just the optional householdId that's stopping my code so far from working, if you could solve that we'll be ready, just within the limits of the current Mealie versioning. We could also totally ignore that field for now, we don't use it.

We also need to add the define_household_support call to the integration at client setup.

@joostlek
Copy link
Owner

Yea I'll be looking into this tonight or tomorrow. Which means we don't make 8.3 but will be included in 2024.9

@andrew-codechimp
Copy link
Contributor Author

The Mealie Add On is now correctly using latest rather than nightly, a restore required for those that upgraded already and that's mentioned in the release but at least its reduced impact for integration users.
Bonus is the Add-On users are now on a Mealie version rather than nightly so no log warnings in future.

@joostlek
Copy link
Owner

Yea I'll also look into helping mealie get a proper nightly versioning. I want to avoid to do a trail and error. This is the first time this happens, but who knows how many more times we have such change?

@joostlek joostlek added the new-feature New features or options. label Aug 30, 2024
@codecov-commenter
Copy link

codecov-commenter commented Aug 30, 2024

Codecov Report

Attention: Patch coverage is 92.59259% with 2 lines in your changes missing coverage. Please review.

Project coverage is 99.07%. Comparing base (1cbf64e) to head (8cd5b7d).

Files with missing lines Patch % Lines
src/aiomealie/mealie.py 90.47% 1 Missing and 1 partial ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main     #236      +/-   ##
==========================================
- Coverage   99.67%   99.07%   -0.60%     
==========================================
  Files           4        4              
  Lines         308      325      +17     
  Branches       34       36       +2     
==========================================
+ Hits          307      322      +15     
- Misses          1        2       +1     
- Partials        0        1       +1     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@joostlek joostlek marked this pull request as ready for review August 30, 2024 12:15
@joostlek
Copy link
Owner

I think I am just going to build a check that will warn users that we will deprecate the older versions once this one is released

@joostlek joostlek merged commit 7e451d9 into joostlek:main Aug 30, 2024
11 checks passed
@andrew-codechimp
Copy link
Contributor Author

Thanks for fixing up those optionals, obvious now I see it!
With most people seeming to be auto-updating with that HA AddOn we should be able to deprecate V1 support pretty quickly and get rid of the cludge.

@andrew-codechimp andrew-codechimp deleted the households branch August 30, 2024 13:04
@github-actions github-actions bot locked and limited conversation to collaborators Sep 1, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
new-feature New features or options.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants