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 initial flask app #2

Merged
merged 24 commits into from
Mar 12, 2024
Merged

Add initial flask app #2

merged 24 commits into from
Mar 12, 2024

Conversation

cbartz
Copy link
Collaborator

@cbartz cbartz commented Mar 8, 2024

Applicable spec: ISD-116 (internal)

Overview

Add the initial flask application. Only the webhook receiving functionality is included. The webhooks are logged into files in a certain directory, which will then be transmitted automatically to Loki using cos integration.

Follow-up PRs will include:

Rationale

A webhook router application is required as defined in the specification.

Checklist

There is no charm yet, so there are no docs on charmhub.

@cbartz cbartz added the complex label Mar 8, 2024
@cbartz cbartz marked this pull request as ready for review March 11, 2024 09:51
@cbartz cbartz requested a review from a team as a code owner March 11, 2024 09:51
@cbartz
Copy link
Collaborator Author

cbartz commented Mar 11, 2024

@weiiwang01 Can you take a look, please? Thanks.

@cbartz cbartz changed the title WIP - Add initial flask app Add initial flask app Mar 11, 2024
src/app.py Outdated Show resolved Hide resolved
src/app.py Outdated Show resolved Hide resolved
Copy link

@jdkandersson jdkandersson left a comment

Choose a reason for hiding this comment

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

I think it looks good once the comments from Weii are addressed :)

Copy link

Test coverage for 4756523

Name         Stmts   Miss Branch BrPart  Cover   Missing
--------------------------------------------------------
src/app.py      32      3      2      1    88%   65-67
--------------------------------------------------------
TOTAL           32      3      2      1    88%

Static code analysis report

Run started:2024-03-12 12:05:53.451122

Test results:
  No issues identified.

Code scanned:
  Total lines of code: 173
  Total lines skipped (#nosec): 3
  Total potential issues skipped due to specifically being disabled (e.g., #nosec BXXX): 0

Run metrics:
  Total issues (by severity):
  	Undefined: 0
  	Low: 0
  	Medium: 0
  	High: 0
  Total issues (by confidence):
  	Undefined: 0
  	Low: 0
  	Medium: 0
  	High: 0
Files skipped (0):

@jdkandersson
Copy link

I'll put the note here as well, we were looking into this, and it seems there is the gunicorn way available as well: https://www.google.com/url?q=https://trstringer.com/logging-flask-gunicorn-the-manageable-way/&sa=D&source=docs&ust=1710249238666567&usg=AOvVaw210jtoz0IaFpGgAiZ48V_m

@cbartz
Copy link
Collaborator Author

cbartz commented Mar 12, 2024

I'll put the note here as well, we were looking into this, and it seems there is the gunicorn way available as well: https://www.google.com/url?q=https://trstringer.com/logging-flask-gunicorn-the-manageable-way/&sa=D&source=docs&ust=1710249238666567&usg=AOvVaw210jtoz0IaFpGgAiZ48V_m

@jdkandersson
Thanks, but regarding the webhook logs, I would like to distinguish between gunicorn/app logs and those to separate them more easily in COS (e.g. to filter just for /var/log/whrouter/*.log filename in Grafana).
Regarding the application logs, this might be the way to go, though I think this should be added once we have the charm code (which is responsible for installing gunicorn and running flask via it).

@cbartz cbartz merged commit 0d43582 into main Mar 12, 2024
20 checks passed
@cbartz cbartz deleted the ISD-1535-initial-flask-app branch March 12, 2024 15:03
@cbartz cbartz mentioned this pull request Mar 27, 2024
6 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants