Skip to content
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

Enhance database operations for better debugging #96

Merged
merged 5 commits into from
Jan 17, 2024

Conversation

deldesir
Copy link
Collaborator

@deldesir deldesir commented Jan 15, 2024

🚀 Pull Request Overview:

This pull request includes the following enhancements to the download task:

  • Improved error handling for download failures with detailed error messages.
  • Enhanced database operations to handle errors and provide clearer messages.
  • Improved retrieval of requested files and shelf title from the SQLite database.

These changes aim to make the download process more robust and provide users with better information about download failures.

📋 Checklist:

  • Tested the changes thoroughly.
  • Checked for backward compatibility (bookshelf feature)

🔗 Related Issues:
Issue #89

📌 Testing scenarios:
See Issue #97

cc @EMG70

@deldesir deldesir added the enhancement New feature or request label Jan 15, 2024
@deldesir deldesir requested a review from holta January 15, 2024 05:30
@deldesir deldesir self-assigned this Jan 15, 2024
@holta
Copy link
Member

holta commented Jan 16, 2024

  1. @deldesir how does this quick test of PR Enhance database operations for better debugging #96 look to you?

    iiab-diagnostics: https://sprunge.us/16LYXQ?en#n-873

  2. What aspects of PR Enhance database operations for better debugging #96 should @EMG70 test most urgently on Tuesday (i.e. in coming hours, if he happens to have time!!)

  3. ASIDE: Are /var/log/calibre-web.log blank lines on 962 and 1001 something we should clean up? (Or is that an upstream Calibre-Web issue we should not get involved with for now?)

@deldesir deldesir marked this pull request as ready for review January 16, 2024 13:22
@deldesir
Copy link
Collaborator Author

  1. @deldesir how does this quick test of PR Enhance database operations for better debugging #96 look to you?
    iiab-diagnostics: https://sprunge.us/16LYXQ?en#n-873

Looks clean to me. With this info, we know which commits/PR is being tested

  1. What aspects of PR Enhance database operations for better debugging #96 should @EMG70 test most urgently on Tuesday (i.e. in coming hours, if he happens to have time!!)

Backward compatibility, ensuring playlists download without an itch.

  1. ASIDE: Are /var/log/calibre-web.log blank lines on 962 and 1001 something we should clean up? (Or is that an upstream Calibre-Web issue we should not get involved with for now?)

Not very important, but we'll keep an eye to it so someday we can look into it.

@holta
Copy link
Member

holta commented Jan 16, 2024

3. What aspects of PR Enhance database operations for better debugging #96 should @EMG70 test most urgently on Tuesday (i.e. in coming hours, if he happens to have time!!)

Backward compatibility, ensuring playlists download without an itch.

@deldesir Are you saying this PR #96 solves... ?

(Thanks for clarifying & explaining so there's no ambiguity!)

@deldesir
Copy link
Collaborator Author

Yes, but not without #99

@holta
Copy link
Member

holta commented Jan 16, 2024

Yes, but not without #99

Motivations behind PR #99 are not (yet!) clear to me and @EMG70:

If it's blocking testing or other PR's (not great, but Ok!) then please prioritize helping everyone in understanding PR #99's motivations, Thank You! 💯

@holta
Copy link
Member

holta commented Jan 17, 2024

PR #99 appears to contain serious problems based on a misunderstanding as to where we are headed:

So hopefully merging this PR #96 can serve as a basis for reworking PR #99 — moving us all (baby steps! one step at a time!) — towards what's actually needs... 🙏

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants