Skip to content

Commit

Permalink
runtime instruction updates and linting
Browse files Browse the repository at this point in the history
  • Loading branch information
hyptocrypto committed Jan 31, 2022
1 parent 40a3581 commit d0b2f68
Show file tree
Hide file tree
Showing 6 changed files with 71 additions and 29 deletions.
2 changes: 2 additions & 0 deletions .gitignore
Original file line number Diff line number Diff line change
@@ -1,3 +1,5 @@
**env/
**venv/
*.env
*.log
*.db
Expand Down
42 changes: 38 additions & 4 deletions RUNTIME_INSTRUCTIONS.md
Original file line number Diff line number Diff line change
@@ -1,19 +1,53 @@
Note:
A few things where unclear for me in the instructions. When changing settings, should only the next calls to the api be based on the new settings, or should the whole app reflect the new settings? Currently its the ladder and all data is reset to be based on the updated settings. Also, the settings file is never actually modified, the state of the settings is just managed in memory. Additionally, the instructions name the db and the virtual environment are listed as 'deliverables'. But from my experience its quite bad practice to upload these. They will be created when running the startup script. I can upload the local copies I have if needed, just not sure that is expected or needed.
## Github
[Me](https://github.com/hyptocrypto)

## Notes
I like to use an ORM when possible. This could run a tiny bit faster with raw sql executions, just not as clean or readable IMO.

A few things where unclear for me in the instructions. When changing settings, should only the next calls to the api be based on the new settings, or should the whole app reflect the new settings? Currently its the ladder and all data is reset to be based on the updated settings. Also, the settings file is never actually modified, the state of the settings is just managed in memory. Additionally, the instructions name the db and the virtual environment are listed as 'deliverables'. But from my experience its quite bad practice to upload these. They will be created when running the startup script. I can upload the local copies I have if needed, just not sure that is expected or needed.

## Security
For best security practices, all credentials are managed as environment variables. So first create a .env file from the template and fill in necessary values.
Something like: cat .env.template >> .env && vim .env
```bash
cat .env.template >> .env && vim .env
```
Also, of course the HTML form implements some CSRF protection. Since its just one simple form I just made it with raw HTML. For anything outside of a simple demo app like this, I would use flask-wtf forms.


## Run steps
macOS & Linux


- /bin/bash startup.sh
or
- make a virtual env and activate
- install deps
- python app.py



Windows


- make a virtual env and activate
- install deps
- python app.py
- python3 app.py


## Libraries
Flask: for the backed<br/>
Flask-wtf: for CSRF <br/>
PeeWee: A lightweight ORM. Cleaner and easier then a bunch of raw SQL.<br/>
Requests: For calling API's and parsing responses<br/>
Dateutil: The datetime format of the the weather api was a little weird. This made it easy to parse into a more readable format. <br/>
Dotenv: To manage env variables<br/>


## Challenge
1. Ensuring that the threaded background worker actually updated the db. Tested and it does. Please let me know if something goes wrong there, threads can be weird thanks to pythons GIL.
2. Spent a short while trying to pass the right data to one of the Alertus endpoints and kept getting 500's. I was just hitting the wrong endpoint to activate an alert.


## Extra
Using threads to speed up I/O calls to API. Also, updating the data at the set check in frequency is handled by a background thread to not block.
Code is linted with isort, flake8, and balck. Depending on flake8 config, it should pass linting tests.
12 changes: 6 additions & 6 deletions app.py
Original file line number Diff line number Diff line change
@@ -1,20 +1,19 @@
import constants
from manager import WeatherDataManager
from flask import Flask, jsonify, redirect, url_for, request, render_template
from flask import Flask, redirect, url_for, request, render_template
from flask_wtf.csrf import CSRFProtect

weather_data_manager = WeatherDataManager()

app = Flask(__name__)
app.config["SECRET_KEY"] = constants.SECRET_KEY
CSRFProtect(app)



@app.route("/error", methods=["GET"])
def error():
return render_template("error.html")


@app.route("/", methods=["GET"])
def data():
location = weather_data_manager.get_location()
Expand All @@ -23,6 +22,7 @@ def data():
data = weather_data_manager.get_latest_data()
return render_template("data.html", data=data, location=location)


@app.route("/settings", methods=["GET", "POST"])
def settings():
# Update settings
Expand All @@ -36,11 +36,11 @@ def settings():
weather_data_manager.update_weather_data()
weather_data_manager.update_alerts()
return redirect(url_for("data"))

if request.method == "GET":
data = weather_data_manager.get_settings()
return render_template("settings.html", data=data)



if __name__ == "__main__":
app.run(host='0.0.0.0', port=5000)

2 changes: 1 addition & 1 deletion constants.py
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
import os

from dotenv import load_dotenv

load_dotenv()
Expand All @@ -23,4 +24,3 @@
ALERTUS_GET_ALERT_API_ENDPOINT = (
"https://demo.alertustech.com/alertusmw/services/rest/presets"
)

2 changes: 1 addition & 1 deletion db.py
Original file line number Diff line number Diff line change
Expand Up @@ -21,5 +21,5 @@ class Weather(Model):
alert_id = IntegerField(null=True)

class Meta:
database = SqliteDatabase(constants.DATABASE_URL)
database = SqliteDatabase(constants.DATABASE_URL, check_same_thread=False)
table_name = "weather"
40 changes: 23 additions & 17 deletions manager.py
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
import logging
from typing import Dict, List, Optional
from concurrent.futures import thread
import threading
import time

Expand All @@ -15,7 +16,7 @@ class WeatherDataManager:
def __init__(self):
"""
Init the manager, set default values,
create db table(if it already exists, this has not effect),
create db table(if it already exists, this has no effect),
and update the db with the latest data.
"""
self.lat = None
Expand All @@ -26,7 +27,7 @@ def __init__(self):
self._init_settings()

# Init db
self._db = SqliteDatabase(constants.DATABASE_URL)
self._db = SqliteDatabase(constants.DATABASE_URL, check_same_thread=False)
self._db.connect()
self._db.create_tables([db.Weather])
self._db.close()
Expand All @@ -37,12 +38,13 @@ def __init__(self):
check_in_worker.daemon = True
check_in_worker.start()

def _check_in(self):
def _check_in(self) -> None:
"""Background worker to updated the db"""
while True:
self.update_weather_data()
time.sleep(int(self.frequency)*60)

def _parse_settings(self):
def _parse_settings(self) -> Dict[str, str]:
"""Parse settings file and return dict containing only lat, lng, thresh, and frequency"""
settings_dict = {}
with open("settings.txt", "r") as f:
Expand All @@ -55,7 +57,7 @@ def _parse_settings(self):
settings_dict[split_line[0]] = split_line[-1]
return {key: value for key, value in settings_dict.items() if key}

def _init_settings(self):
def _init_settings(self) -> None:
"""Call the weather api and set class attrs"""
settings = self._parse_settings()
lat, lng = settings.get("latitude"), settings.get("longitude")
Expand All @@ -80,9 +82,9 @@ def _init_settings(self):
self.frequency = int(freq)
self.grid_url = grid_url

def update_weather_data(self):
def update_weather_data(self) -> None:
"""
Call the weather gird endpoint, and fill the db with the latest 10 records of 3 hour segments
Call the weather gird endpoint, and fill the db with the latest 10 records of 3 hour segments.
"""
resp = requests.get(
headers={"Accept": "application/cap+xml"}, url=self.grid_url
Expand All @@ -91,7 +93,8 @@ def update_weather_data(self):
forecast_list = resp.json().get("properties").get("periods")
grouped_forecasts = zip(*[iter(forecast_list[:30])] * 3)
for forecast1, forecast2, forecast3 in grouped_forecasts:
# Its not defined which of the three hours the timestap should refer to. Currently its the first
# Its not defined which of the three hours the timestap should refer to. Currently its the first.
# Only update the db if the forcast does not already exit.
if (
not db.Weather.select()
.where(
Expand All @@ -110,10 +113,13 @@ def update_weather_data(self):
second_forecast=forecast2.get("temperature"),
third_forecast=forecast3.get("temperature"),
)
self._send_alert(instance)
# Start each call to api in its own thread to speed things up a little
threaded_worker = threading.Thread(target=self._send_alert, args=(instance,))
threaded_worker.daemon = True
threaded_worker.start()


def update_settings(self, lat, lng, freq, thresh):
def update_settings(self, lat, lng, freq, thresh) -> None:
"""Update new cls attrs. If lat and lng have been changed, updated the gird_url"""
if float(lat) != float(self.lat) or float(lng) != float(self.lng):
resp = requests.get(
Expand All @@ -131,7 +137,7 @@ def update_settings(self, lat, lng, freq, thresh):
self.thresh = int(thresh)
self.frequency = int(freq)

def _send_alert(self, model: db.Weather):
def _send_alert(self, model: db.Weather) -> None:
"""
Check if any of the forcats temps are greater then the thresh,
if so, update the model instance
Expand All @@ -156,13 +162,13 @@ def _send_alert(self, model: db.Weather):
model.alert_id = None
model.save()

def get_latest_data(self):
def get_latest_data(self) -> List[Dict]:
return [model_to_dict(d) for d in db.Weather.select().order_by(db.Weather.id.desc()).limit(10)]

def _parse_timestamp(self, timestamp):
return dateutil.parser.parse(timestamp).strftime("%Y-%m-%d %H:%M")

def _get_first_preset_alert(self):
def _get_first_preset_alert(self) -> Optional[Dict]:
resp = requests.get(
headers={'Content-type': 'application/json'},
url=constants.ALERTUS_GET_ALERT_API_ENDPOINT,
Expand All @@ -175,7 +181,7 @@ def update_alerts(self):
for forecast in db.Weather.select():
self._send_alert(forecast)

def _set_location(self, resp):
def _set_location(self, resp) -> None:
if not resp.ok:
self.city = None
self.state = None
Expand All @@ -184,15 +190,15 @@ def _set_location(self, resp):
self.city = data.get("city")
self.state = data.get("state")

def get_settings(self):
def get_settings(self) -> Dict:
return {
"latitude": self.lat,
"longitude": self.lng,
"threshold": self.thresh,
"frequency": self.frequency,
}

def get_location(self):
def get_location(self) -> Dict:
return {
"city": self.city,
"state": self.state
Expand Down

0 comments on commit d0b2f68

Please sign in to comment.