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

Remove nginx rewrite rules, add static and media paths to nginx #76

Merged

Conversation

MrApplejuice
Copy link
Contributor

@MrApplejuice MrApplejuice commented Jul 24, 2024

Required when babybuddy/babybuddy#852 is merged.
Addresses issue babybuddy/babybuddy#844

I moved the rewrites (not literally but conceptually) into the django middleware in babybuddy/babybuddy#852. The rewrite rules in nginx are gone.

I also added /static and /media path routing to the nginx configuration because ... that is how it is supposed to work in django if I am not mistaken. I am not quite sure how the /static content was served before, but /media content did not seem to be served at all, right?

@OttPeterR
Copy link
Owner

OttPeterR commented Jul 24, 2024

Media was broken before, correct.

Another issue I'm hitting when trying to update to the latest BB version is issues with the rarer architectures, so not amd64 or aarch64, but the other three. According to dockerhub those downloads are pretty rare. But I bring this up because we may have to drop support for those on the next release.

See the build checks that are failing in #73

@MrApplejuice
Copy link
Contributor Author

Hey!

Dropping armv7 and armhf is probably a bit of a bummer. I think that this will drop support for raspberry pi's which, I think are still a quite popular low-cost way of running home assistant...

When quickly checking the logs, it seems that the ninja install fails? That should not be a hard dependency afterall, I think. I think we can work around that, either by installing ninja via a side-channel (and not pip) or by using a different build-toolchain for whatever is trying to use ninja... I can maybe have a look later this week!

@OttPeterR
Copy link
Owner

I'd really appreciate it if you took a look at it. When I last tried it was like playing whack a mole after each error was fixed a new one showed up. But hey it's worth a shot!

As a heads up for your dev testing, the GitHub builds for one of those other architectures (I forget which) can take up to an hour - so put in a change then go do something else for a bit lol

@cdubz
Copy link

cdubz commented Jul 24, 2024

Just chiming in that I’d very much like to avoid dropping support for any archs. Baby Buddy already does some stuff weirdly to ensure support for smaller user bases. I’m traveling for another 1.5 weeks but I can also help look at that aspect when I’m back., if needed.

@MrApplejuice
Copy link
Contributor Author

Hi... sorry for my sudden absence. I got ill, and did spent my time in bed instead of fixing issues. I am back out of bed now, nut need to get going again. Going to be picking this up again now...

@MrApplejuice
Copy link
Contributor Author

@OttPeterR - I revived my work on babybuddy/babybuddy#852 . If that one gets merged, this one should be merged, too, before the next home assistant release that includes patch 852. I quickly tested the container builds for homeassistant as well and all cross-compiled versions work at the moment!

@OttPeterR
Copy link
Owner

Thats awesome news! To clarify, do you need me to do anything on my end or are your tests able to be put into this PR?

@MrApplejuice
Copy link
Contributor Author

Hi!

I do not quite understand what you mean with:

or are your tests able to be put into this PR

  • Are you referring to the failing GitHub actions-stuff? That seems to be GitHub actions tool chain related. I can have a quick look if you want in the next few days.
  • Or do you mean tests for the babybuddy app in some form? There are no automated ones at the moment, right? Also they are really hard to write if we wanted to test against home assistant, because that requires some hard provisioning magic to spin everything up and then come up with solid automated tests. I did most of this testing manually with virtual machines...

do you need me to do anything

For that: Not really. This needs to be merged after babybuddy/babybuddy#852 was merged so that the homeassistant addon still works. That is all.

@OttPeterR OttPeterR mentioned this pull request Nov 5, 2024
@OttPeterR
Copy link
Owner

sorry for the late follow-up...
Also I have no idea what I was saying about "tests" so disregard that entirely lol

Those GitHub actions fail to build when the base HA image isnt the most up to date and some SSL dependencies are deprecated. Once I get #73 taken care of to make all the architectures build properly, this can go right in!

@OttPeterR OttPeterR changed the base branch from main to v2.4.0 November 6, 2024 13:42
@OttPeterR
Copy link
Owner

OttPeterR commented Nov 6, 2024

Can you update this to the new head?
https://github.com/OttPeterR/addon-babybuddy/tree/v2.4.0
It should build properly after that and I'll get it merged in for a new release!

edit: never mind it can just go right in 😅

@OttPeterR OttPeterR merged commit f9b4401 into OttPeterR:v2.4.0 Nov 6, 2024
0 of 5 checks passed
@MrApplejuice
Copy link
Contributor Author

I was about to acknowledge that I read your post and could have acted on it this evening (CEST) or tomorrow morning. Thanks for merging! Very self sufficient. 🤞 that it works as tested locally!

OttPeterR added a commit that referenced this pull request Nov 6, 2024
* BB 2.6.2, HA base 16.3.4 

* --only-binary

* add C MAX_PATH limit

* prefer-binary

* Direct pandas install

* bump

* base to 16.1.1

* bump to 16.1.2

* 16.2.0 base

* bump to BB2.6.1 and HA base 16.3.4

* remove pandas dep

* BB 2.6.2

* Replace nginx rewrite rules, add static-paths for static and media queries (#76)

---------

Co-authored-by: Peter Ott <[email protected]>
Co-authored-by: Paul K. Gerke <[email protected]>
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.

3 participants