Open
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 (
|
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
Comment on lines
101
to
127
| ): | ||
| 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.
🤖 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.
Suggested change
| ): | |
| 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 |
Comment on lines
136
to
+159
| ): | ||
| 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.
🤖 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 AttributeErrors.
Suggested change
| ): | |
| 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, | |
| ) |
Comment on lines
+3
to
+7
|
|
||
| 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.
🤖 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.
Suggested change
| 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 |
Comment on lines
+15
to
+24
|
|
||
| router = APIRouter() | ||
|
|
||
|
|
||
| @router.post( | ||
| "/post_verse_of_the_day/", | ||
| tags=[ | ||
| "social_v3", | ||
| ], | ||
| ) |
There was a problem hiding this comment.
🤖 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.
Comment on lines
+25
to
+32
| 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.
🤖 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.
Comment on lines
+33
to
+40
|
|
||
| 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.
🤖 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.
Comment on lines
+1
to
+16
| 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.
🤖 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.
Suggested change
| 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>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
4 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary by OpenAI
New Features:
SANSKRIT_RECITATION_HOSTenvironment variable and added support for Sanskrit recitation audio files.API_V3_STRconstant toSettingsclass, enabling a new API version./post_verse_of_the_day/for posting verse of the day on Twitter.Refactor:
schemas.GitaVerseinstead ofmodels.GitaVerse.GitaVerse,GitaTranslation, andGitaCommentaryto use lazy loading.Chore:
python3.8 or abovetopython3.10 or above.