search

haroldtreen / epub-press

📚 Web service for creating ebooks from collections of websites.
https://epub.press
GNU General Public License v3.0
327 stars 59 forks source link

Add Calibre as default for MOBI conversion #78

Closed sanujar closed 2 years ago

sanujar commented 3 years ago

PR should resolve #77.

The commits in this pull request download and use Calibre as the default for MOBI conversion, with the option to switch to Kindlegen by changing an build ARG in the Dockerfile.

This also adds an ENV_VAR to start a SMTP connection with TLS (leaving it blank (default) will lead to a plain-text connection with an attempt to use STARTTLS). Documentation from Nodemailer for this is here.

Documentation for these changes has also been added to the README.

sanujar commented 2 years ago

This looks super cool! Congrats on getting this done despite the unfamiliarity with everything 🎉🎉🎉🎉🎉.

Thank you! Again, I really do appreciate all the help, and I'm glad I was able to contribute to epub-press.

Also curious to push this to the epub press staging server and test it out there + still need to run this locally. I'll follow-up when I wrap up with that 👍

Absolutely! I'd appreciate some very thorough testing of everything, because while it all seemed to work on my machine™, I'd feel much better if this code were put through its paces somewhere else first before being pushed to prod.

sanujar commented 2 years ago

I also added documentation to the README on the env MAX_NUM_SECTIONS added by #69. Slightly confused with how this hardcoded constant in routes/api/books-beta.js would work/interact though:

https://github.com/haroldtreen/epub-press/blob/52188ee63f7ab9bb0078f0b6149e9841e2ee94f5/routes/api/books-beta.js#L13

haroldtreen commented 2 years ago

Sorry for the delay! Going to try and push this to the staging environment and can see how it fares there 👍

haroldtreen commented 2 years ago

Deployed this to staging 🎉. Unfortunately looks like something is not working...

image

Actually think I've figured it out though! Looks like my suggestion for && vs. | was misguided and the latter is what is needed for it to work 🙈.

If you point your extension at https://stage.epub.press or visit that directly - you should see your version running 👌

sanujar commented 2 years ago

If you point your extension at https://stage.epub.press or visit that directly - you should see your version running 👌

Just did, and as far as I can tell, everything works absolutely fine! A MOBI file with chapters generated and downloaded, and my mod to the Chrome extension to add author metadata worked too, even if it's still throwing that error.

haroldtreen commented 2 years ago

Thanks for the quick fix of the dockerfile! I'm pushing the current state of the world to stage. 👍

Once I confirm that fixes it - I might push another to check switching to kindlegen with the environment variable - but other than that I think this is looking good 🤩

sanujar commented 2 years ago

Finished testing!

  • Email is still working
  • Convert is still working
  • Toggle for reverting to Kindlegen is working

Congrats @sanujar on adding such a great change 🎉. Really appreciate all the thought and effort into this!

Let's ship it 🚀

Thank you, @haroldtreen! I really do appreciate your going through this and reviewing everything, despite epub-press not being a primary focus for you anymore. Thank you too for all your help, walking me through how this code is supposed to work as well as syntax for node.js. I really don't think I'd have been able to do this without your help.

I really did enjoy contributing to epub-press, which is something I regularly use, and I love having been able to help improve it.