Arreglar !lyrics, que nunca funcionó con la lyricsgenius fijada - #38
Merged
Merged
Conversation
Every lookup died in the client constructor:
WARNING loopify.lyrics: Genius lookup failed for '...':
Genius.__init__() got an unexpected keyword argument 'quiet'
`quiet` belonged to lyricsgenius 2.x. The pinned 3.12.2 has no such argument and
no `verbose` either — it prints nothing of its own, so there was never anything
to silence and the argument is simply dropped rather than replaced.
The TypeError happened while building the client, so it took out every query,
and the command answered "Couldn't find lyrics for ..." — which reads like a
failed search rather than a broken build. That is why it went unnoticed: the
command appeared to work and simply never found anything.
The bug predates the cleanup in #32. What changed is that #32 replaced
`print(f"[Lyrics] Error: {e}")` with a real logger call, so the reason finally
showed up in journalctl with a level and a logger name attached.
Two related problems fixed alongside it:
- No timeout was passed, so a lookup could pin an executor thread indefinitely —
and a pinned executor thread delays the loop's shutdown, the ceiling #35 was
about. It is now bounded at 10s (the library default is 5s).
- A missing token built a client anyway, and lyricsgenius then falls back to
$GENIUS_ACCESS_TOKEN and raises KeyError. `fetch` now gives up before that,
which is also the honest answer for a user typing !lyrics on a deployment
without a token.
`skip_non_songs=True` and `remove_section_headers=False` match the library's
current defaults, but stay explicit because the behaviour is relied on: Genius
indexes tracklists and credits pages, and the `[Chorus]` markers are wanted in
the embed. Tests assert both rather than trusting the defaults to hold.
tests/test_lyrics_api.py is new — this module had no tests at all, which is
exactly how a TypeError in a constructor lived here unnoticed. Nothing in them
talks to Genius; they check that the installed library accepts what we pass, that
a missing token is handled before it can raise, that the call is bounded, and
that the blocking search never runs on the event loop.
Verified against the real API from the server: "Bohemian Rhapsody" / "Queen"
comes back with its section headers intact, and a nonsense query returns None
with no traceback.
Tests: 344, up from 332.
Closes #37
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
1 participant
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.
Cierra #37.
344 tests (antes 332).
!lyricsno funcionaba, y no había funcionado nuncacon la versión de lyricsgenius que fija
requirements.txt.Qué pasaba
Cada consulta moría al construir el cliente:
quietera de la serie 2.x. La 3.12.2 fijada no lo tiene, y tampoco tieneverbose: no imprime nada por su cuenta, así que nunca hubo nada quesilenciar. El argumento se elimina, no se sustituye.
Por qué pasó desapercibido
El
TypeErrorocurría al construir, así que se llevaba toda consulta, y elcomando respondía "Couldn't find lyrics for ...". Eso se lee como una búsqueda
sin resultados, no como una construcción rota — el comando parecía funcionar y
simplemente no encontrar nada nunca.
El bug es anterior al #32. Lo que cambió es que el #32 sustituyó
print(f"[Lyrics] Error: {e}")por una llamada real al logger, y entonces elmotivo apareció en
journalctlcon nivel y nombre de logger.Dos problemas relacionados, arreglados de paso
timeout. Una consulta podía clavar un hilo del executorindefinidamente — y un hilo del executor clavado retrasa el cierre del loop,
el techo del que iba el Un reinicio tras reproducir música agota TimeoutStopSec y dispara una alerta falsa #35. Ahora está acotado a 10 s (el defecto de la
librería es 5 s).
$GENIUS_ACCESS_TOKENy lanzaKeyError. Ahorafetchse rinde antes, que esademás la respuesta honesta para quien teclea
!lyricsen un despliegue sintoken.
Sobre los valores por defecto
skip_non_songs=Trueyremove_section_headers=Falsecoinciden con los defectosactuales de la librería, pero se pasan explícitos porque dependemos de ese
comportamiento: Genius indexa tracklists y páginas de créditos, y las marcas
[Chorus]se quieren en el embed. Los tests los afirman en vez de confiar en quelos defectos no cambien.
Tests
tests/test_lyrics_api.pyes nuevo — este módulo no tenía ningún test, que esexactamente cómo un
TypeErroren un constructor pudo vivir aquí sin que nadiese enterase. Ninguno habla con Genius; comprueban que la librería instalada
acepta lo que le pasamos, que la falta de token se maneja antes de que pueda
lanzar, que la llamada está acotada, y que la búsqueda bloqueante nunca corre en
el event loop.
Cómo probarlo
Verificado contra la API real desde el servidor:
En Discord, tras desplegar:
Y en
journalctlno debe quedar ningúnGenius lookup failed.