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

feat(api): include external port option #1516

Merged
merged 2 commits into from
Aug 23, 2020
Merged

feat(api): include external port option #1516

merged 2 commits into from
Aug 23, 2020

Conversation

jef
Copy link
Contributor

@jef jef commented Jul 3, 2020

Description

This should help with a fix when downloading from a URL that goes through a reverse proxy, especially through Docker.

As there could be port mappings within Docker (12345:8083), then a reverse proxy: (54321 -> 12345), and even potential router port forwarding (34125 -> 54321). This allows the user to make API calls correctly to links like api_endpoint for Kobo.

@jef
Copy link
Contributor Author

jef commented Jul 3, 2020

If we don't think this could go under the main Server Configuration (since it seems to be mostly related to Kobo), we can put this option under Feature Configuration -> Enable Kobo sync as well. But I figured that this could help other features in the future.

Signed-off-by: Jef LeCompte <jeffreylec@gmail.com>
@jef jef mentioned this pull request Jul 4, 2020
@OzzieIsaacs OzzieIsaacs merged commit 843279b into janeczku:master Aug 23, 2020
@OzzieIsaacs
Copy link
Collaborator

I had time to double check it, there is indeed not chance for detecting the port. One thing I don't conclude with you is when it is needed. In my opinion it is needed in case of port forwarding, and (now) in case of direct connection with non standard ports. For reverse proxy configurations, the code is detecting the reverse proxy and the malformed kobo request isn't getting to calibre-web.
But anyhow, I did some small changes to your code:

  • I moved the setting near to kobo setting, as it's only relevant in case kobo sync is used
  • I removed the reboot necessary in the save section
  • I changed the text according to my conclusion when it is needed.

Thanks for the work

@jef
Copy link
Contributor Author

jef commented Aug 24, 2020

You bet! Thanks for the update and merge. You have much more insight to how calibre-web works, so I really appreciate the help! I'll pull down the latest this evening and hopefully can close out a few issues.

@jef jef deleted the jef/download-kobo branch August 24, 2020 14:43
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants