Solution #652
Solution #652
Conversation
Dockerfile
Outdated
| FROM python:3.11.6-alpine3.18 | ||
|
|
||
| ENV PYTHONUNBUFFERED 1 | ||
|
|
||
| WORKDIR app/ | ||
|
|
||
| COPY . . | ||
| RUN pip install -r requirements.txt | ||
|
|
||
| CMD ["python", "app/main.py"] |
There was a problem hiding this comment.
The Dockerfile does not follow best practices
https://docs.docker.com/build/building/best-practices/
app/main.py
Outdated
| result = requests.get(f"{URL}key={API_KEY}&q={FILTERING}").json() | ||
|
|
||
| city = result["location"]["name"] | ||
| country = result["location"]["country"] | ||
| local_time = result["location"]["localtime"] | ||
| temperature = result["current"]["temp_c"] | ||
| condition = result["current"]["condition"]["text"] | ||
|
|
||
| print("Performing request to Weather API for city Paris...") | ||
| print( | ||
| f"{city}/{country} {local_time} " | ||
| f"Weather: {temperature} Celsius, {condition}" | ||
| ) |
There was a problem hiding this comment.
Please add try exception block, also you could split into functions
app/main.py
Outdated
| API_KEY = os.getenv("api_key") | ||
| URL = "https://api.weatherapi.com/v1/current.json?" | ||
| FILTERING = "Paris" |
There was a problem hiding this comment.
If you set api_key as an environment variable, why not do the same for URL and FILTERING?
There was a problem hiding this comment.
It's our task condition:
You need to generate API_KEY for using Weather API, but it shouldn't be hard-coded in main.py.
You should use environment variables for that purpose. So, read API_KEY from environment. And to pass environment variables to docker container use -e flag.
There was a problem hiding this comment.
First, in the naming convention, you should use os.getenv("API_KEY"). Second, if you pass API_KEY as an environment variable, ensure it is correctly set. Regarding other fields, I’m confused about why I can’t decide which city I want to see. I want to have a single configuration where I can input the desired city. The same applies to the URL—what happens if it changes? What should we do then?
Dockerfile
Outdated
| FROM python:3.11.6-alpine3.18 | ||
|
|
||
| WORKDIR /app | ||
| COPY requirements.txt . | ||
| RUN pip install --no-cache-dir -r requirements.txt | ||
|
|
||
| RUN adduser -D myuser | ||
| USER myuser | ||
|
|
||
| COPY . . | ||
|
|
||
| CMD ["python3", "app/main.py"] |
There was a problem hiding this comment.
# syntax=docker/dockerfile:1
# Comments are provided throughout this file to help you get started.
# If you need more help, visit the Dockerfile reference guide at
# https://docs.docker.com/go/dockerfile-reference/
# Want to help us make this template better? Share your feedback here: https://forms.gle/ybq9Krt8jtBL3iCk7
ARG PYTHON_VERSION=3.12
FROM python:${PYTHON_VERSION}-slim as base
# Prevents Python from writing pyc files.
ENV PYTHONDONTWRITEBYTECODE=1
# Keeps Python from buffering stdout and stderr to avoid situations where
# the application crashes without emitting any logs due to buffering.
ENV PYTHONUNBUFFERED=1
WORKDIR /app
# Create a non-privileged user that the app will run under.
# See https://docs.docker.com/go/dockerfile-user-best-practices/
ARG UID=10001
RUN adduser \
--disabled-password \
--gecos "" \
--home "/app" \
--shell "/sbin/nologin" \
--no-create-home \
--uid "${UID}" \
appuser
# Switch to the non-privileged user to run the application.
USER appuser
COPY ./app /app/
# Expose the port that the application listens on.
EXPOSE 8000
# Використовуємо змінну середовища для вибору запуску
# Run the application.
CMD python app/main.py
app/main.py
Outdated
| API_KEY = os.getenv("api_key") | ||
| URL = "https://api.weatherapi.com/v1/current.json?" | ||
| FILTERING = "Paris" |
There was a problem hiding this comment.
First, in the naming convention, you should use os.getenv("API_KEY"). Second, if you pass API_KEY as an environment variable, ensure it is correctly set. Regarding other fields, I’m confused about why I can’t decide which city I want to see. I want to have a single configuration where I can input the desired city. The same applies to the URL—what happens if it changes? What should we do then?
app/main.py
Outdated
| def get_weather() -> None: | ||
| # write your code here | ||
| pass | ||
| result = requests.get(f"{URL}key={API_KEY}&q={FILTERING}").json() | ||
|
|
||
| print(parse_weather_data(result)) |
There was a problem hiding this comment.
Why try exception for parse_weather_data but not for get_weather?
app/main.py
Outdated
| def get_weather() -> None: | ||
| # write your code here | ||
| pass | ||
| result = requests.get(f"{URL}key={API_KEY}&q={FILTERING}").json() |
There was a problem hiding this comment.
using .json in the end is risky. Better approach to split it into 2 variables.
app/main.py
Outdated
| def get_weather() -> None: | ||
| # write your code here | ||
| pass | ||
| result = requests.get(f"{URL}key={API_KEY}&q={FILTERING}").json() |
There was a problem hiding this comment.
Result name: Does this describe the result of the request? Partially yes, but it does not describe the data inside it. A better name would be weather_response.
| URL = "https://api.weatherapi.com/v1/current.json?" | ||
| FILTERING = "Paris" |
| f"{city}/{country} {local_time} " | ||
| f"Weather: {temperature} Celsius, {condition}" | ||
| ) | ||
| except KeyError as e: |
There was a problem hiding this comment.
use more descriptive naming e -> exc or e -> exception:
I prefer exc
| @@ -0,0 +1,2 @@ | |||
| venv | |||
| Dockerfile | |||
No description provided.