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

Change default web root to sabnzbd #2

Closed
wants to merge 1 commit into from
Closed

Change default web root to sabnzbd #2

wants to merge 1 commit into from

Conversation

arlyon
Copy link

@arlyon arlyon commented Sep 30, 2018

Sabnzbd uses /sabnzbd/ as the default web_root, so it would make sense to change the default behaviour of this library to reflect that. Additionally, an empty web_root will error out because it produces a malformed url: http://hostname:8080//api as opposed to the expected http://hostname:8080/api. For reference, the default sabnzbd api url is http://hostname:8080/sabnzbd/api.

@@ -11,7 +11,7 @@ def __init__(self, base_url, api_key, web_root=None,
if web_root is not None:
web_root = '{}/'.format(web_root.strip('/'))
else:
web_root = ''
web_root = 'sabnzbd'
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Did you test this change? Just looking at it, this will create a url like http://host/sabnzbdapi.

Copy link
Author

@arlyon arlyon Oct 3, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, locally, however I notice now the version on pypi is not the same as the latest version on github (one commit behind) which means I was looking at one version on my machine and making changes to another; this pull request is meaningless. Sorry for taking up your time, it was a long day and it explains why I was so confused haha

I will download the latest version from git instead and see if that fixes things.

Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sounds good. I will get the new version on PyPi today so it can be updated in home assistant too

@jeradM
Copy link
Owner

jeradM commented Oct 3, 2018

I'm not sure this is the right approach here. SABnzbd responds to api calls at /api and /sabnzbd/api so it shouldn't matter. In addition, forcing the webroot to sabnzbd might not actually be what someone wants, they might actually want it to be / if they are behind a proxy or something. If someone using this library really needs the webroot to be sabnzbd, they can just pass that in. Also, if the webroot should be /, you shouldn't pass in an empty string, you should leave the default or pass in None

@arlyon arlyon closed this Oct 3, 2018
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.

None yet

2 participants