-
-
Notifications
You must be signed in to change notification settings - Fork 35
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
V2 #92
base: main
Are you sure you want to change the base?
V2 #92
Conversation
…PI and have made necessary changes in following files .env.example - added recitation_url host api/api_v2/endpoints/gita.py - modified resolver function to include recitation url * models/schemas.py - added the recitation url field
Feat--recitations
Revert "Feat--recitations"
🚀 feat--Sanskrit audio recitation
🚀 feat--Gita-Transliteration
…PI and have made necessary changes in following files .env.example - added recitation_url host api/api_v2/endpoints/gita.py - modified resolver function to include recitation url * models/schemas.py - added the recitation url field
🤖 OpenAI High-Level SummaryThe changes introduce a new version (v3) of the API, adding features like posting verses on Twitter and handling Sanskrit recitations. The codebase now supports Python 3.10 and uses GitHub Actions for PR reviews. Database operations have been optimized to handle updates and avoid duplicates. Environment variables have been updated, including the addition of a testing API key.
Chat with 🤖 OpenAI Bot (
|
): | ||
verses = ( | ||
db.query(models.GitaVerse) | ||
.options( | ||
joinedload(models.GitaVerse.commentaries), | ||
joinedload(models.GitaVerse.translations), | ||
) | ||
.order_by(models.GitaVerse.id.asc()) | ||
.filter(models.GitaVerse.chapter_number == chapter_number) | ||
.all() | ||
) | ||
if verses is None: | ||
raise HTTPException(status_code=404, detail="Verse not found") | ||
return verses | ||
res = [] | ||
for verse in verses: | ||
res.append( | ||
schemas.GitaVerse( | ||
id=verse.id, | ||
verse_number=verse.verse_number, | ||
chapter_number=verse.chapter_number, | ||
slug=verse.slug, | ||
text=verse.text, | ||
transliteration=verse.transliteration, | ||
word_meanings=verse.word_meanings, | ||
sanskrit_recitation_url=verse.sanskrit_recitation_url, | ||
translations=verse.translations.all(), | ||
commentaries=verse.commentaries.all(), | ||
) | ||
) | ||
return res | ||
|
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.
🤖 OpenAI
The logic for querying the verses and handling the case when no verses are found is incorrect. The SQLAlchemy all()
method returns an empty list when no results are found, not None
. Therefore, the condition if verses is None:
will never be true even if no verses are found. You should check if the list is empty instead.
): | |
verses = ( | |
db.query(models.GitaVerse) | |
.options( | |
joinedload(models.GitaVerse.commentaries), | |
joinedload(models.GitaVerse.translations), | |
) | |
.order_by(models.GitaVerse.id.asc()) | |
.filter(models.GitaVerse.chapter_number == chapter_number) | |
.all() | |
) | |
if verses is None: | |
raise HTTPException(status_code=404, detail="Verse not found") | |
return verses | |
res = [] | |
for verse in verses: | |
res.append( | |
schemas.GitaVerse( | |
id=verse.id, | |
verse_number=verse.verse_number, | |
chapter_number=verse.chapter_number, | |
slug=verse.slug, | |
text=verse.text, | |
transliteration=verse.transliteration, | |
word_meanings=verse.word_meanings, | |
sanskrit_recitation_url=verse.sanskrit_recitation_url, | |
translations=verse.translations.all(), | |
commentaries=verse.commentaries.all(), | |
) | |
) | |
return res | |
verses = ( | |
db.query(models.GitaVerse) | |
.order_by(models.GitaVerse.id.asc()) | |
.filter(models.GitaVerse.chapter_number == chapter_number) | |
.all() | |
) | |
if not verses: | |
raise HTTPException(status_code=404, detail="Verse not found") | |
res = [] | |
for verse in verses: | |
res.append( | |
schemas.GitaVerse( | |
id=verse.id, | |
verse_number=verse.verse_number, | |
chapter_number=verse.chapter_number, | |
slug=verse.slug, | |
text=verse.text, | |
transliteration=verse.transliteration, | |
word_meanings=verse.word_meanings, | |
sanskrit_recitation_url=verse.sanskrit_recitation_url, | |
translations=verse.translations.all(), | |
commentaries=verse.commentaries.all(), | |
) | |
) | |
return res |
): | ||
verse = ( | ||
db.query(models.GitaVerse) | ||
.options( | ||
joinedload(models.GitaVerse.commentaries), | ||
joinedload(models.GitaVerse.translations), | ||
) | ||
.filter( | ||
models.GitaVerse.chapter_number == chapter_number, | ||
models.GitaVerse.verse_number == verse_number, | ||
) | ||
.first() | ||
) | ||
|
||
if verse is None: | ||
raise HTTPException(status_code=404, detail="Verse not found") | ||
return verse | ||
return schemas.GitaVerse( | ||
id=verse.id, | ||
verse_number=verse.verse_number, | ||
chapter_number=verse.chapter_number, | ||
slug=verse.slug, | ||
text=verse.text, | ||
transliteration=verse.transliteration, | ||
word_meanings=verse.word_meanings, | ||
sanskrit_recitation_url=verse.sanskrit_recitation_url, | ||
translations=verse.translations.all(), | ||
commentaries=verse.commentaries.all(), | ||
) |
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.
🤖 OpenAI
Similar to the previous issue, the SQLAlchemy first()
method returns None
when no results are found. So, the condition if verse is None:
is correct in this case. However, you're calling .all()
on verse.translations
and verse.commentaries
, which could potentially return None
if there are no translations or commentaries. It would be better to handle these cases to avoid potential AttributeError
s.
): | |
verse = ( | |
db.query(models.GitaVerse) | |
.options( | |
joinedload(models.GitaVerse.commentaries), | |
joinedload(models.GitaVerse.translations), | |
) | |
.filter( | |
models.GitaVerse.chapter_number == chapter_number, | |
models.GitaVerse.verse_number == verse_number, | |
) | |
.first() | |
) | |
if verse is None: | |
raise HTTPException(status_code=404, detail="Verse not found") | |
return verse | |
return schemas.GitaVerse( | |
id=verse.id, | |
verse_number=verse.verse_number, | |
chapter_number=verse.chapter_number, | |
slug=verse.slug, | |
text=verse.text, | |
transliteration=verse.transliteration, | |
word_meanings=verse.word_meanings, | |
sanskrit_recitation_url=verse.sanskrit_recitation_url, | |
translations=verse.translations.all(), | |
commentaries=verse.commentaries.all(), | |
) | |
verse = ( | |
db.query(models.GitaVerse) | |
.filter( | |
models.GitaVerse.chapter_number == chapter_number, | |
models.GitaVerse.verse_number == verse_number, | |
) | |
.first() | |
) | |
if verse is None: | |
raise HTTPException(status_code=404, detail="Verse not found") | |
translations = verse.translations.all() if verse.translations else [] | |
commentaries = verse.commentaries.all() if verse.commentaries else [] | |
return schemas.GitaVerse( | |
id=verse.id, | |
verse_number=verse.verse_number, | |
chapter_number=verse.chapter_number, | |
slug=verse.slug, | |
text=verse.text, | |
transliteration=verse.transliteration, | |
word_meanings=verse.word_meanings, | |
sanskrit_recitation_url=verse.sanskrit_recitation_url, | |
translations=translations, | |
commentaries=commentaries, | |
) |
|
||
from fastapi import APIRouter, Depends, HTTPException, Response | ||
from sqlalchemy.orm import Session | ||
|
||
from bhagavad_gita_api.api import deps |
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.
🤖 OpenAI
While these imports are fine, it's a good practice to group and order your imports according to PEP8 guidelines. This helps in maintaining the code readability.
from fastapi import APIRouter, Depends, HTTPException, Response | |
from sqlalchemy.orm import Session | |
from bhagavad_gita_api.api import deps | |
from datetime import date | |
import logging | |
from fastapi import APIRouter, Depends, HTTPException, Response | |
from sqlalchemy.orm import Session | |
from bhagavad_gita_api.api import deps | |
from bhagavad_gita_api.models import gita as models |
|
||
router = APIRouter() | ||
|
||
|
||
@router.post( | ||
"/post_verse_of_the_day/", | ||
tags=[ | ||
"social_v3", | ||
], | ||
) |
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.
🤖 OpenAI
The router
object is created but not used immediately. It would be better to move this closer to where it's first used (line 19) to improve readability.
async def post_instagram(db: Session = Depends(deps.get_db)): | ||
verse_of_day = ( | ||
db.query(models.VerseOfDay) | ||
.filter( | ||
models.VerseOfDay.date == date.today(), | ||
) | ||
.first() | ||
) |
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.
🤖 OpenAI
This block of code queries for the verse of the day. However, there is no error handling if the database query fails. Consider adding a try-except block around the database operations to handle any potential exceptions.
|
||
if verse_of_day: | ||
verse = ( | ||
db.query(models.GitaVerse) | ||
.join(models.GitaTranslation) | ||
.filter(models.GitaVerse.id == verse_of_day.verse_order) | ||
.first() | ||
) |
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.
🤖 OpenAI
This block of code fetches the verse corresponding to the verse of the day. Similar to the previous comment, consider adding error handling for database operations.
import enum | ||
import logging | ||
import random | ||
from datetime import date | ||
from typing import List | ||
|
||
from fastapi import APIRouter, Depends, HTTPException, Response | ||
from sqlalchemy import func, or_ | ||
from sqlalchemy.orm import Session, joinedload | ||
|
||
from bhagavad_gita_api.api import deps | ||
from bhagavad_gita_api.models import gita as models | ||
from bhagavad_gita_api.models import schemas | ||
|
||
logger = logging.getLogger("api") | ||
logger.setLevel(logging.DEBUG) |
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.
🤖 OpenAI
The logger is set up at the module level, which is a good practice. However, it's recommended to use __name__
as the name of the logger to get the context of where the logging is happening.
import enum | |
import logging | |
import random | |
from datetime import date | |
from typing import List | |
from fastapi import APIRouter, Depends, HTTPException, Response | |
from sqlalchemy import func, or_ | |
from sqlalchemy.orm import Session, joinedload | |
from bhagavad_gita_api.api import deps | |
from bhagavad_gita_api.models import gita as models | |
from bhagavad_gita_api.models import schemas | |
logger = logging.getLogger("api") | |
logger.setLevel(logging.DEBUG) | |
logger = logging.getLogger(__name__) |
Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
Summary by OpenAI
New Features:
SANSKRIT_RECITATION_HOST
environment variable and added support for Sanskrit recitation audio files.API_V3_STR
constant toSettings
class, enabling a new API version./post_verse_of_the_day/
for posting verse of the day on Twitter.Refactor:
schemas.GitaVerse
instead ofmodels.GitaVerse
.GitaVerse
,GitaTranslation
, andGitaCommentary
to use lazy loading.Chore:
python3.8 or above
topython3.10 or above
.