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 docs for fetching token using container #780

Merged
merged 1 commit into from
Nov 3, 2023

Conversation

KapJI
Copy link
Collaborator

@KapJI KapJI commented Nov 3, 2023

I messed up with #772

Credits to @Brephlas

@KapJI KapJI added the documentation Improvements or additions to documentation label Nov 3, 2023
Copy link
Owner

@leikoilja leikoilja left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks @KapJI for fixing it & @Brephlas for contribution!

Add instructions for docker container for token retrieval
@KapJI KapJI merged commit 8a9abd3 into leikoilja:master Nov 3, 2023
5 checks passed
@KapJI KapJI deleted the brephlas-docs branch November 3, 2023 15:11
@owenvoke
Copy link

owenvoke commented Nov 4, 2023

I had a couple of questions about this Docker image. Is the Dockerfile available anywhere? 👀

And also, if this is solely for this purpose, would it make more sense for the CMD to run the get_tokens.py script, rather than bash so you don't have to connect to the image and leave it running in the background in a detached state?

That way it would just be a case of running this, rather than 3 commands.

docker run --rm -it breph/ha-google-home_get-token:latest

@Brephlas
Copy link
Contributor

Brephlas commented Nov 4, 2023

It is now :) https://github.com/Brephlas/dockerfile_breph-ha-google-home_get-token/tree/master

Regarding the second question: I like the idea and played a little bit with this. Unfortunately, it seems it is not possible to run interactive scripts using this method. At least when asking for user input, the script fails.
So I think we have two options:

  1. stick to the current flow
  2. change it, but require to pass username and password using -e docker environment variables. But I am not sure if I like that one because the credentials are then inside of the history

@owenvoke
Copy link

owenvoke commented Nov 5, 2023

Hi @Brephlas, I've just tested it locally (changing CMD bash to CMD python3 get_tokens.py) and it seems to work perfectly with docker run --rm -it [image]. 🤔 I'm using macOS, and iTerm2.

Preview video

asciicast

Also related to this image, I wonder if we could change to using Alpine to reduce the image size. 🤷🏻

FROM alpine:3.18

WORKDIR /app

RUN apk add py3-pip
COPY ./get_tokens.py .
COPY ./requirements.txt .
RUN python3 -m pip install -r ./requirements.txt

CMD python3 get_tokens.py

This reduces the image from 606MB to 96.7MB 🙂

I guess device_id, master_token, and access_token would probably all make sense as having configurable by Docker secrets.

Happy to move this discussion to your repo though, if that makes more sense.

@Brephlas
Copy link
Contributor

Brephlas commented Nov 5, 2023

Thanks @owenvoke. Apparently, the issue was sitting in front of the screen, I missed the -it parameter 😅
It now worked in my local tests and I've already updated the image to alpine and pushed it to dockerhub.
Will update the docs of this repo in a sec

Regarding the docker secrets we can move the discussion to my repo. However, until now I have no experience with docker secrets

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
documentation Improvements or additions to documentation
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants