Add lb search support by deldesir · Pull Request #140 · iiab/calibre-web

deldesir · 2024-03-22T09:18:48Z

🚀 Pull Request Overview:
With this PR, lb-wrapper can be used with the search command provided a search term is specified as an argument. For example:

lb-wrapper search "your search term"

This will execute the xklb search command with the provided search term

📋 Checklist:

Tested the changes on Ubuntu 24.04 with the following playlist: https://www.youtube.com/playlist?list=PLsFuJoB0uOFVqdL-C70fZCpNUl46SrjYd (3 videos) using xklb v2.5.019

root@box:~# lb-wrapper search welcome 
/usr/local/bin/lb
2024-03-22 08:10:07 - [Info] Running xklb command: lb search /library/calibre-web/xklb-metadata.db welcome
3 captions
Borges et la Machine à Fiction (CMS #1) - /library/calibre-web/Artificialis Code/Borges et la Machine à Fiction (CMS #1) (6)/Borges et la Machine à Fiction (CMS #1) - Artificialis Code.mp4
    0:01 friends fans of friends fans of artificial intelligence welcome to a new,. artificial intelligence welcome to a new,
         artificial intelligence welcome to a new, somewhat special video you can

I-JEPA: révolutionner l'IA par l'apprentissage auto-supervisé (5mn1p) - /library/calibre-web/Artificialis Code/I-JEPA_ révolutionner l'IA par l'apprentissage auto-supervisé (5mn1p) (5)/I-JEPA_ révolutionner l'IA par l'apprenti - Artificialis Code.mp4
    0:01 dear friend artificial intelligence enthusiast dear friend artificial intelligence enthusiast welcome to a new. welcome to a
         new welcome to a new 5-minute episode for a paper

Sur les dangers des perroquets stochastiques (5mn1p) - /library/calibre-web/Artificialis Code/Sur les dangers des perroquets stochastiques (5mn1p) (7)/Sur les dangers des perroquets stochastiqu - Artificialis Code.mp4
    0:02 friend artificial intelligence lover friend artificial intelligence lover welcome to this new video. welcome to this new
         video welcome to this new video today it's the first episode

yt_dlp_rc: 0
2024-03-22 08:10:08 - [Info] lb-wrapper's xklb command (search) completed successfully.

📌 Testing scenarios:
See Issue #97

cc @EMG70

holta · 2024-03-22T14:04:41Z

Does this require... ?

PR Download "top 100 videos" from YouTube channel or playlist, sorted by views-per-day #139

deldesir · 2024-03-22T14:06:41Z

No, ~~but #141 will require it.~~

deldesir · 2024-03-28T15:23:08Z

Related to #152

deldesir

This PR introduces the "search" argument which is used in background when a user enter something in the search field in Calibre-Web UI. This can be tested in terminal with lb-wrapper search <terms>.

holta · 2024-05-02T16:46:04Z

scripts/lb-wrapper

+    # adjust above line to also reroute the return code of the underlying command as a line saying "return code: 0" or "return code: 1" or "return code: 2" etc.
+    > >(while read -r line; do if [[ $line == downloading* || $line =~ \[https://.*\]:* ]] || [[ $line == "yt_dlp_rc: "* ]]; then echo "$line"; else log "Info" "$line"; fi; done) \
+    2> >(while read -r line; do if [[ $line == downloading* || $line =~ \[https://.*\]:* ]]; then echo "$line"; else log "Debug" "$line" 1>&2; fi; done; echo "yt_dlp_rc: $?" 1>&2) &


I'm trying to understand this.

@deldesir is the following closer to the truth, of what we're trying to accomplish?

Suggested change

# adjust above line to also reroute the return code of the underlying command as a line saying "return code: 0" or "return code: 1" or "return code: 2" etc.

> >(while read -r line; do if [[ $line == downloading* || $line =~ \[https://.*\]:* ]] || [[ $line == "yt_dlp_rc: "* ]]; then echo "$line"; else log "Info" "$line"; fi; done) \

2> >(while read -r line; do if [[ $line == downloading* || $line =~ \[https://.*\]:* ]]; then echo "$line"; else log "Debug" "$line" 1>&2; fi; done; echo "yt_dlp_rc: $?" 1>&2) &

# adjust above line to also reroute the return code of the underlying command as a line saying "xklb_rc: 0" or "xklb_rc: 1" or "xklb_rc: 2" etc.

> >(while read -r line; do if [[ $line == downloading* || $line =~ \[https://.*\]:* || $line == "xklb_rc: "* ]]; then echo "$line"; else log "Info" "$line"; fi; done) \

2> >(while read -r line; do if [[ $line == downloading* || $line =~ \[https://.*\]:* ]]; then echo "$line"; else log "Debug" "$line" 1>&2; fi; done; echo "xklb_rc: $?" 1>&2) &

I was trying to make lb-wrapper to report underlying yt-dlp return code. We already had $rc for xklb.

Return code $? on Line 69 is arising from what command exactly?

(This definitely doesn't look like yt-dlp's return code.)

In any case, please clear up what $? is really doing here, and why?
(And how this was tested please if possible!)

It's the exit status of the last underlying command (yt-dlp) executed by xklb.

I could see it's yt-dlp because I could reroute its output. This exit status code is not doing anything useful here. It was me debugging yt-dlp and I forgot to remove this code.

This exit status code is not doing anything useful here. It was me debugging yt-dlp and I forgot to remove this code.

Please remove any temporary code and confirm.

Please also fix Lines 56, 58 and 60 which contain (former variable) XKLB_EXECUTABLE.

Thanks!

Temporary code removed.

holta · 2024-05-02T16:48:19Z

scripts/lb-wrapper

+case $rc in
+    0)
+        log "Info" "lb-wrapper's xklb command (${XKLB_INTERNAL_CMD}) completed successfully."
+        exit 0
+        ;;
+    1)
+        log "Error" "Error $rc occurred while running lb-wrapper's xklb commands."
+        exit 1
+        ;;
+    *)
+        log "Error" "Unknown error occurred while running lb-wrapper's xklb commands. Return code: $rc"
+        exit $rc
+        ;;
+esac


Is a 3-way case statement really necessary?

(What's so special about Return Code 1?)

Can we instead simplify, as follows?

Suggested change

case $rc in

0)

log "Info" "lb-wrapper's xklb command (${XKLB_INTERNAL_CMD}) completed successfully."

exit 0

;;

1)

log "Error" "Error $rc occurred while running lb-wrapper's xklb commands."

exit 1

;;

*)

log "Error" "Unknown error occurred while running lb-wrapper's xklb commands. Return code: $rc"

exit $rc

;;

esac

if [ $rc -eq 0 ]; then

log "Info" "lb-wrapper's xklb command (${XKLB_INTERNAL_CMD}) completed successfully."

else

log "Error" "Error $rc occurred while running lb-wrapper's xklb commands."

exit $rc

fi

Nothing special. But exit 0 is missing in the if statement. So I suggest:

Suggested change

case $rc in

0)

log "Info" "lb-wrapper's xklb command (${XKLB_INTERNAL_CMD}) completed successfully."

exit 0

;;

1)

log "Error" "Error $rc occurred while running lb-wrapper's xklb commands."

exit 1

;;

*)

log "Error" "Unknown error occurred while running lb-wrapper's xklb commands. Return code: $rc"

exit $rc

;;

esac

if [ $rc -eq 0 ]; then

log "Info" "lb-wrapper's xklb command (${XKLB_INTERNAL_CMD}) completed successfully."

else

log "Error" "Error $rc occurred while running lb-wrapper's xklb commands."

exit $rc

fi

exit $rc is the same as exit 0 on Line 87, and doesn't do anything useful.

(Programs always exit with return code 0, regardless, when they complete successfully.)

Recap: please remove exit $rc on Line 87, ok?

Ok. Thanks for clarifying.

holta · 2024-05-02T21:31:27Z

Looks good, and sorry if that was a bit of a distraction at the bottom of scripts/lb-wrapper !

@deldesir, the key question (above) is what $? is doing on Line 69, when you have a sec: (explain &/or show test results?!)

Add lb search support #140 (comment)

holta · 2024-05-02T21:41:31Z

@deldesir please resolve the simple merge conflict, arising from PR #159 having just been merged?

Thanks!!

deldesir · 2024-05-02T21:51:41Z

Done

holta · 2024-05-04T01:11:29Z

What testing does this most need?

(Should this be tested together with... ?)

PR Support searching videos [subtitles, captions, transcripts?] #152

Add lb search support

52b2f86

deldesir marked this pull request as draft March 22, 2024 09:18

deldesir added 2 commits March 22, 2024 05:27

Support searching through subtitles

7cbc16e

revert search.py changes

b5c72ca

deldesir requested a review from holta March 22, 2024 12:13

deldesir self-assigned this Mar 22, 2024

deldesir added the enhancement New feature or request label Mar 22, 2024

deldesir marked this pull request as ready for review March 22, 2024 12:13

holta mentioned this pull request Mar 22, 2024

Support search through subtitles [WIP] #141

Closed

Align with the "Top 100" feature

1e257e8

deldesir mentioned this pull request Mar 28, 2024

Support searching videos [subtitles, captions, transcripts?] #152

Open

1 task

deldesir commented May 2, 2024

View reviewed changes

holta reviewed May 2, 2024

View reviewed changes

Use if statement to handle exit code

9506372

Merge branch 'master' into deldesir-patch-1

e02b552

deldesir added 2 commits May 3, 2024 20:38

Remove XKLB_EXECUTABLE variable

e28f600

Remove temporary code

b10d99c

holta mentioned this pull request May 28, 2024

Captions/Subtitle Search feature is broken chapmanjacobd/library#39

Closed

holta merged commit 916963a into master May 29, 2024

Conversation

deldesir commented Mar 22, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

holta commented Mar 22, 2024

Uh oh!

deldesir commented Mar 22, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

deldesir commented Mar 28, 2024

Uh oh!

deldesir left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

deldesir May 2, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

deldesir May 2, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

holta commented May 2, 2024

Uh oh!

holta commented May 2, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

deldesir commented May 2, 2024

Uh oh!

holta commented May 4, 2024

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

deldesir commented Mar 22, 2024 •

edited

Loading

deldesir commented Mar 22, 2024 •

edited

Loading

deldesir left a comment •

edited

Loading

deldesir May 2, 2024 •

edited

Loading

deldesir May 2, 2024 •

edited

Loading

holta commented May 2, 2024 •

edited

Loading