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 devcontainer and pave the way for ingress access #106

Merged
merged 4 commits into from
Aug 28, 2023

Conversation

siku2
Copy link
Contributor

@siku2 siku2 commented Aug 24, 2023

Closes: #22

image

The only way to see that it's working in this image is because the styles are loaded. But trust me, the buttons are working too.


This PR enables support for the Home Assistant ingress by reading the contents of the standard X-Ingress-Path header. This header is also used by many other reverse proxies, so this will allow them to function correctly on sub paths as well.
If there's no reverse proxy in front of emhass, it will work like before, so this won't break the non-addon setup.

There are also two other changes in this PR:

  • I added a devcontainer that allows new contributors to get into the project more quickly. It's very bare-bones at the moment, but it can run the emhass web server without problems (which is how I was working on this project).
  • Removed duplication from the "requirements*.txt" files by referencing the main dependencies in the web server one.

I'm of course happy to remove these changes from this PR if you wish.

There will be a separate PR over at davidusb-geek/emhass-add-on that's required before the ingress actually works.

@siku2 siku2 changed the title Add devcontainer and pave the for ingress access Add devcontainer and pave the way for ingress access Aug 24, 2023
@davidusb-geek
Copy link
Owner

Again thanks for this.
Could you please add a quick explanation on how to run the devcontainer for testing.
I use the official Home Assistant devcontainer for testing the add-on part which actually runs a complete HA test instance. I run it from VSCode with no problem, is this something similar?

@codecov
Copy link

codecov bot commented Aug 25, 2023

Codecov Report

Merging #106 (50d382c) into master (d2d247c) will not change coverage.
The diff coverage is n/a.

@@           Coverage Diff           @@
##           master     #106   +/-   ##
=======================================
  Coverage   88.15%   88.15%           
=======================================
  Files           6        6           
  Lines        1418     1418           
=======================================
  Hits         1250     1250           
  Misses        168      168           

@siku2
Copy link
Contributor Author

siku2 commented Aug 25, 2023

@davidusb-geek, yes, but it doesn't include HA (yet :P). For now it gives you a full Python development environment that contains all the necessary dependencies to work on emhass.

If you want to test it out:

  1. Head over to my fork, press on the friendly green "Code" button and create a new codespace:
    image
    Codespaces are based on devcontainers so this will give you a vscode instance in the browser loaded with the devcontainer that I added.

    You can, of course, also open the project locally in vscode, but testing it with github is less of a hassle (and more secure).

  2. Now you can just run cd src && python -m emhass.web_server. The server won't actually start as-is, because it doesn't find the configuration files in /app, but if you adjust the paths you will be able to fix it.

@siku2
Copy link
Contributor Author

siku2 commented Aug 25, 2023

Btw, there's a lot more that I want to improve wrt. the development flow and especially the user experience. I really want to use this add-on personally, but right now it's not quite in a state where I feel comfortable doing so. Keep in mind I'm only saying that because I'm actually willing to make the changes necessary to get to that point.

I'll try to find some time this weekend to write up an issue containing my thoughts to get the discussion going. I hope you're open to this idea, @davidusb-geek?

@davidusb-geek davidusb-geek merged commit 91319e2 into davidusb-geek:master Aug 28, 2023
@davidusb-geek
Copy link
Owner

Thanks for these changes and the instructions to test the devcontainer. I've just merged this.

@davidusb-geek
Copy link
Owner

Btw, there's a lot more that I want to improve wrt. the development flow and especially the user experience. I really want to use this add-on personally, but right now it's not quite in a state where I feel comfortable doing so. Keep in mind I'm only saying that because I'm actually willing to make the changes necessary to get to that point.

I'll try to find some time this weekend to write up an issue containing my thoughts to get the discussion going. I hope you're open to this idea, @davidusb-geek?

Yes of course this is open for contribution and improvements.
And we can discuss what you have in mind.

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.

Feature request: Access EMHASS web interface via ingress
2 participants