-
Notifications
You must be signed in to change notification settings - Fork 12
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
Create python-app.yml #27
base: main
Are you sure you want to change the base?
Conversation
…amers into ci-cd-app-engine
…amers into ci-cd-app-engine
…amers into ci-cd-app-engine
* Create python-app.yml * formatting * reformatting code * Delete python-app.yml * adding first test * adding first test * requirements * Update ci.yml * Update ci.yml * refactoring env vars * formatting * adding env for test * using env.test * using env.test * adding env.test * Update ci.yml * adding env.test * adding env.test * code coverage
@@ -0,0 +1,18 @@ | |||
ENV=dev | |||
CLIENT_ID=6zakmdaqzkjqga9fmmsq922uq3tgw0 | |||
CLIENT_SECRET=qsgqic56t5mrip34lz5mcbr9o09ns3 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
remember to refresh all this secrets :)
@@ -32,56 +37,55 @@ | |||
) | |||
|
|||
|
|||
AUTH0_DOMAIN = 'zapperson.us.auth0.com' | |||
AUTH0_DOMAIN = "zapperson.us.auth0.com" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
its not better to leave it to ".env"
try: | ||
token = decode_jwt(Authorization) | ||
nickname = token['https://brstreamers.dev/nickname'] | ||
if(nickname == user_login): | ||
nickname = token["https://brstreamers.dev/nickname"] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What you think about this approach?
than you can leave the URL on .env or other place than you think could be better :)
URL="https://brstreamers.dev"
nickname = token[f"{URL}/nickname}
|
||
app_public.add_middleware( | ||
CORSMiddleware, | ||
allow_origins=origins, | ||
allow_origins=origins, | ||
allow_credentials=True, | ||
allow_methods=["*"], |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Maybe in future review to not use "*". but alow only what we really want.
@@ -26,14 +29,16 @@ | |||
|
|||
@app_public.get("/streams", response_model=List[StreamViewModel]) | |||
@cache(expire=60) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Create a variable EXPIRE_TIME = 60
and use in all
@cache(expire=EXPIRE_TIME)
return get_streamers() | ||
async def streams(): | ||
twitch_service = TwitchService(twitch) | ||
return twitch_service.get_streamers() | ||
|
||
|
||
@app_public.get("/vods", response_model=List[VodViewModel]) | ||
@cache(expire=60) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
same than above
if os.environ["ENV"] == "prod": | ||
uvicorn.run( | ||
"main:app", | ||
host="0.0.0.0", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
is this host="0.0.0.0"
really necessary?
res = ( | ||
User.update( | ||
{ | ||
User.instagram: user.instagram, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
future issue: its not better to leave it in one list?
linkedin, github, twitter, discord ...
@@ -4,9 +4,9 @@ | |||
config = dotenv_values(".env") | |||
|
|||
|
|||
twitch = Twitch(config['CLIENT_ID'], config['CLIENT_SECRET']) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is this file necessary?
|
||
try: | ||
streamers = get_users_by_name(stream_users) | ||
for s in streamers: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
its not better to use:
for streamer in streamers:
instead of s
?
vods_model.append(stream) | ||
try: | ||
streamers = get_users_by_name(vod_users) | ||
for s in streamers: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
same than line 49.
No description provided.