Improve polling and logging #86

deldesir · 2024-01-03T04:58:30Z

This PR has following changes:
- Enhanced logging.
- Added more detailed error handling for database operations and subprocess execution.
- Wrapped database operations in a try-except block for better error handling.
- Ensured consistent progress updates.

Tested on Ubuntu 24.04 (10.8.0.70)

holta · 2024-01-03T05:16:17Z

Log progress while downloading chapmanjacobd/library#26
Use existing job queuing system for downloads #9
- Redirect to tasks route not working [or better yet educate users, so they click on "Tasks"] #20
- Download tasks don't seem to run simultaneously, causing delays #58
Add xklb version [and more, to friendlier & more efficient logging] #70
- Consolidate /var/log/calibre-web.log and /var/log/xklb.log ? Using systemd's journalctl? #68
"Download to IIAB" worked 5 out of 15 tries (trying to d/l individual YouTube videos) — "Tasks" view screenshots show the 10 failures [e.g. formerly live videos] #84
- "Download to IIAB" failures must be shown in "Tasks" view with explanation [e.g. "Media check failed (will try again later)" "Failed reading header" with .mhtml] [e.g. YouTube_Movies_TV channel encrypted w/ browser-based DRM?] #85

holta · 2024-01-03T05:44:02Z

cps/tasks/download.py

+                        if "creating" in line:
+                            self.progress += 0.025
+                        log.info("Download progress: %s", self.progress)
+                        if "Running ANALYZE" in line:
+                            self.progress = 0
+                            line = p.stdout.readline()
+                        if "extracting" in line:
+                            self.message = "Downloading media..."
+                            self.progress += 0.077
+                        elif "downloading" in line:
+                            self.progress += 0.077
+                        elif "download" in line:
+                            self.progress += 0.077
+                        elif "merging" in line:
+                            self.progress += 0.077
+                        elif "adding" in line:
+                            self.progress += 0.075
+                            break
+                        else:
+                            self.progress += 0.077
+                        log.info("Download progress: %s", self.progress)


These lines need more explaining:

How were the 8 numbers chosen?

Why is 0.077 used 5 times, and 0.075 used once?

Why is a break statement now needed with "adding", unlike in the past?

Does 0.075 do anything at all, if it breaks out of the while loop, and is overwritten soon after (on Line 102) with self.progress = 1

Can the solitary test for "creating" (on Line 48) possibly be made a part of the if-elif-elif-elif-elif-else structure further below?

I refactored it and got rid of pattern matching. The logging will be more aligned with #68. However it builds on a customization made to xklb's underlying yt_dlp code, in which I use progress hooks.

builds on a customization made to xklb's underlying yt_dlp code

What xklb or yt-dlp code are you talking about, to help others understand?

in which I use progress hooks

What line number in your code? (Thanks!)

I am talking about this changes:
deldesir/library@f923477

deldesir/library@f923477

@chapmanjacobd just FYI! 🏗️

I am talking about this changes: chapmanjacobd/[email protected]:library:main

@deldesir please clarify:

Are you asking everyone to change how they install IIAB with Calibre-Web ?

Are you asking them to customize xklb code, after pipx install xklb ?

Please spell out precisely (AI²) whatever you are suggesting (Thanks!)

AI² = Articulate Assumptions, Intentions & Intuition

No need to customize xklb. IIAB install instructions remain the same.

This will build upon progress hooks

xklb can be too verbose

holta · 2024-01-03T21:08:19Z

Is this revised PR tested? (Thanks!)

deldesir · 2024-01-04T01:36:59Z

Is this revised PR tested? (Thanks!)

It's being actively tested on 10.8.0.70. An example test is to try to download this video https://www.youtube.com/watch?v=r2wBcCIlMGw. It will fail, but thanks to enhanced logging we now know why it fails. The error is read from the database too, as suggested by @chapmanjacobd

holta · 2024-01-04T06:44:53Z

QUESTION: Why are commits 322046c and e52ae66 reducing logging verbosity?

Many bugs are very urgently in need of more detailed (or at least most illustrative) logging — rather than less!

Just a few examples here:

"Download to IIAB" of Vimeo video ("Staff Pick" URL or regular URL) does not download [and 1 YouTube video too, then 2 YouTube videos worked] #78
"Download to IIAB" worked 5 out of 15 tries (trying to d/l individual YouTube videos) — "Tasks" view screenshots show the 10 failures [e.g. formerly live videos] #84
"Download to IIAB" failures must be shown in "Tasks" view with explanation [e.g. "Media check failed (will try again later)" "Failed reading header" with .mhtml] [e.g. YouTube_Movies_TV channel encrypted w/ browser-based DRM?] #85

holta · 2024-01-04T06:48:50Z

@codewiz is this logging design solid?

Or getting closer?

Show errors in tasks view

holta · 2024-01-05T00:19:47Z

@deldesir please help explain this PR to everyone (AI²).

As well as answering the questions from 27h, 17h and 10h ago — allowing @EMG70 and others to understand what's happening here and test:

Thanks!

holta · 2024-01-05T19:06:50Z

Just FYI @EMG70 is busy today — but he would like guidance on how to test this PR #86 over the wkd &/or beyond.

Alongside the just merged:

Enhance verbosity with detailed download progress information chapmanjacobd/library#28

deldesir · 2024-01-06T01:14:58Z

QUESTION: Why are commits 322046c and e52ae66 reducing logging verbosity?

Many bugs are very urgently in need of more detailed (or at least most illustrative) logging — rather than less!

Just a few examples here:

* ["Download to IIAB" of Vimeo video ("Staff Pick" URL or regular URL) does not download [and 1 YouTube video too, then 2 YouTube videos worked] #78](https://github.com/iiab/calibre-web/issues/78)

* ["Download to IIAB" worked 5 out of 15 tries (trying to d/l individual YouTube videos) — "Tasks" view screenshots show the 10 failures #84](https://github.com/iiab/calibre-web/issues/84)

* ["Download to IIAB" failures must be shown in "Tasks" view with explanation [e.g. "Media check failed (will try again later)" "Failed reading header" with .mhtml] #85](https://github.com/iiab/calibre-web/issues/85)

I was testing log levels and wanted to find the optimal one to capture efficiently progress information. I will keep lb dl log level to -vv because it's the minimum to set to make this functional.

Summary of this PR:

Meaningful errors are logged both in /var/log/calibre-web.log and in the tasks' message column.
Download progress information is captured using a temporary polling file - Capturing with "Named Pipe (FIFO)" failed.
Messages and progress are reported dynamically in the tasks' view.

NB. A new related PR will be shipped to add a "retry" button in tasks' "actions column.

holta · 2024-01-09T02:51:46Z

Just FYI I'm not in favor of this PR auto-closing these 3 issues:

Consolidate /var/log/calibre-web.log and /var/log/xklb.log ? Using systemd's journalctl? #68
Add xklb version [and more, to friendlier & more efficient logging] #70
"Download to IIAB" worked 5 out of 15 tries (trying to d/l individual YouTube videos) — "Tasks" view screenshots show the 10 failures [e.g. formerly live videos] #84

(The above 3 issues are certainly related, but they were carefully written up to address larger issues like systemd's journalctl logging that will certainly be needed later, but those things are likely far above and beyond this current PR #86, which needs to remain tightly focused.)

holta · 2024-01-09T03:09:51Z

FYI @deldesir 955716e was an accidentally destructive commit, reversing several commits from earlier today.

holta · 2024-01-09T04:04:32Z

Positive: While I don't yet understand this PR's core assumptions...

(it definitely needs more AI² = Articulate Assumptions, Intentions & Intuition)

...the good news is that today it's starting to converge — to become progressively more Clearly Explained + Cleanly Understandable.

Add "format options" instructions to test large size videos. Co-authored-by: A Holt <[email protected]>

Co-authored-by: A Holt <[email protected]>

holta · 2024-01-10T14:24:55Z

FYI this PR #86 inspired...

PR Suggested improvements for scripts/lb-wrapper [refactor logging & subprocess substitution, etc] #87

The goal (as explained there) is smaller PR's and smaller (sometimes even Artisanal?!) ideas:

TDD momentum = Architectural Humility + Collaboration Authenticity

🪜

https://en.wikipedia.org/wiki/A_journey_of_a_thousand_miles_begins_with_a_single_step

holta · 2024-01-11T13:36:03Z

FYI this PR #86 inspired...

PR Suggested improvements for scripts/lb-wrapper #87

Progress ongoing, e.g. in @deldesir's just-merged:

PR Refactor pattern matching [cps/tasks/download.py: parse yt-dlp output for download progress percentage] #88

holta · 2024-01-12T05:11:44Z

TDD momentum = Architectural Humility + Collaboration Authenticity

🪜

https://en.wikipedia.org/wiki/A_journey_of_a_thousand_miles_begins_with_a_single_step

More progress ongoing:

PR Enhance download task messages for improved information when downloading a video #89
PR 3 regex's (STDERR, STDIN, Python) to parse yt-dlp download progress, for live display in "Tasks" view #93

holta · 2024-01-14T05:16:56Z

Progress continues:

PR Update tasks messages based on the current status #90

holta · 2024-05-02T16:57:53Z

@deldesir should this PR be closed?

(If so, please explain what is replacing and why, Thanks!)

deldesir · 2024-05-02T17:04:49Z

Yes. It was abandoned in favor of smaller PRs. Already merged #87 was the first one spinned off it.

deldesir added 2 commits January 2, 2024 23:52

Improve polling

6ba7544

Improve polling

8c00a12

deldesir added the enhancement New feature or request label Jan 3, 2024

deldesir self-assigned this Jan 3, 2024

scripts/lb-wrapper.greedy: lb tubeadd ... -vv && lb dl ... -vvv

0a1ca8c

holta reviewed Jan 3, 2024

View reviewed changes

deldesir marked this pull request as draft January 3, 2024 12:45

deldesir added 3 commits January 3, 2024 14:41

Improve download task

3509efc

Prepare lb-wrapper

322046c

This will build upon progress hooks

Prepare lb-wrapper.greedy

e52ae66

xklb can be too verbose

This comment was marked as duplicate.

Sign in to view

deldesir added 2 commits January 4, 2024 01:19

Enhance polling

423c846

Capture progress

ca46406

deldesir requested a review from holta January 4, 2024 06:24

deldesir marked this pull request as ready for review January 4, 2024 06:25

Show error msg

3b9d16d

Show errors in tasks view

deldesir marked this pull request as draft January 4, 2024 12:08

Format messages

558536a

deldesir marked this pull request as ready for review January 4, 2024 12:21

holta mentioned this pull request Jan 5, 2024

Enhance verbosity with detailed download progress information chapmanjacobd/library#28

Merged

deldesir marked this pull request as draft January 5, 2024 19:00

Increase log level

6844322

deldesir mentioned this pull request Jan 9, 2024

"Download to IIAB" failures must be shown in "Tasks" view with explanation [e.g. "Media check failed (will try again later)" "Failed reading header" with .mhtml] [e.g. YouTube_Movies_TV channel encrypted w/ browser-based DRM?] #85

Closed

Log xklb version

955716e

deldesir and others added 4 commits January 9, 2024 07:07

Untrack lb-wrapper.greedy

2242353

Update scripts/lb-wrapper

251af1d

Add "format options" instructions to test large size videos. Co-authored-by: A Holt <[email protected]>

Delete scripts/lb-wrapper.greedy

a33f04b

Update scripts/lb-wrapper

b7eef86

Co-authored-by: A Holt <[email protected]>

holta mentioned this pull request Jan 10, 2024

Suggested improvements for scripts/lb-wrapper [refactor logging & subprocess substitution, etc] #87

Merged

holta mentioned this pull request Jan 11, 2024

Refactor pattern matching [cps/tasks/download.py: parse yt-dlp output for download progress percentage] #88

Merged

4 tasks

deldesir mentioned this pull request Jan 12, 2024

Redirect to STDOUT instead of STDERR: scripts/lb-wrapper #92

Closed

holta mentioned this pull request Jan 12, 2024

3 regex's (STDERR, STDIN, Python) to parse yt-dlp download progress, for live display in "Tasks" view #93

Merged

holta closed this May 2, 2024

deldesir deleted the deldesir-polling branch July 1, 2024 21:42

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Improve polling and logging #86

Improve polling and logging #86

deldesir commented Jan 3, 2024

holta commented Jan 3, 2024 •

edited

Loading

holta Jan 3, 2024 •

edited

Loading

deldesir Jan 3, 2024

holta Jan 3, 2024

deldesir Jan 4, 2024 •

edited

Loading

holta Jan 4, 2024 •

edited by deldesir

Loading

holta Jan 4, 2024

deldesir Jan 6, 2024

holta commented Jan 3, 2024

This comment was marked as duplicate.

deldesir commented Jan 4, 2024 •

edited

Loading

holta commented Jan 4, 2024

holta commented Jan 4, 2024

holta commented Jan 5, 2024 •

edited

Loading

holta commented Jan 5, 2024

deldesir commented Jan 6, 2024

holta commented Jan 9, 2024 •

edited

Loading

holta commented Jan 9, 2024

holta commented Jan 9, 2024

holta commented Jan 10, 2024

holta commented Jan 11, 2024

holta commented Jan 12, 2024

holta commented Jan 14, 2024

holta commented May 2, 2024

deldesir commented May 2, 2024

Improve polling and logging #86

Improve polling and logging #86

Conversation

deldesir commented Jan 3, 2024

holta commented Jan 3, 2024 • edited Loading

holta Jan 3, 2024 • edited Loading

Choose a reason for hiding this comment

deldesir Jan 3, 2024

Choose a reason for hiding this comment

holta Jan 3, 2024

Choose a reason for hiding this comment

deldesir Jan 4, 2024 • edited Loading

Choose a reason for hiding this comment

holta Jan 4, 2024 • edited by deldesir Loading

Choose a reason for hiding this comment

holta Jan 4, 2024

Choose a reason for hiding this comment

deldesir Jan 6, 2024

Choose a reason for hiding this comment

holta commented Jan 3, 2024

This comment was marked as duplicate.

deldesir commented Jan 4, 2024 • edited Loading

holta commented Jan 4, 2024

holta commented Jan 4, 2024

holta commented Jan 5, 2024 • edited Loading

holta commented Jan 5, 2024

deldesir commented Jan 6, 2024

holta commented Jan 9, 2024 • edited Loading

holta commented Jan 9, 2024

holta commented Jan 9, 2024

holta commented Jan 10, 2024

holta commented Jan 11, 2024

holta commented Jan 12, 2024

holta commented Jan 14, 2024

holta commented May 2, 2024

deldesir commented May 2, 2024

holta commented Jan 3, 2024 •

edited

Loading

holta Jan 3, 2024 •

edited

Loading

deldesir Jan 4, 2024 •

edited

Loading

holta Jan 4, 2024 •

edited by deldesir

Loading

deldesir commented Jan 4, 2024 •

edited

Loading

holta commented Jan 5, 2024 •

edited

Loading

holta commented Jan 9, 2024 •

edited

Loading