-
Notifications
You must be signed in to change notification settings - Fork 1.2k
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
Support subpath in url for Nightscout follower #3089
base: master
Are you sure you want to change the base?
Conversation
Is there any Nightscout server that cannot produce a hostname without a subpath? |
I'm not sure what you mean but the library used, retrofit, makes sure the baseurl will always end with a slash so it will be safe to use the relative path. |
Nightscout needs a server. user.heroku.com or user.chickenkiller.com for example. I am asking why would anyone have a subpath? |
Nightscout needs a server indeed, but since it's open source you can decide yourself how you host it right? I'm not really sure why you want to restrict choices? |
The reason xDrip still exists and is supported is that a few volunteers have spent a lot of time on it and continue to support it. Is there a reason you cannot set up your path as user.domain.com without a subpath? |
I would like to ask my question differently to hopefully help clarify my point in case I am not clear already. I understand that if there is a hostname that contains a subpath, currently, xDrip has a problem and this PR will resolve it. I'm not disputing that. But, I wonder why there should be any hostname that contains a subpath. 1- If someone has the option to choose a hostname with or without a subpath, why would they choose to have a subpath? What advantage would it offer over a simple hostname with no subpath? 2- Is there a scenario where a user has no choice and for whatever reason they have to have a subpath? What is that scenario? 3- It is possible that someone has already created a Nightsocut setup and have given you a hostname and you want to follow them. But, the hostname they have given you contains a subpath. Is that the case? Would you please ask them why they chose a subpath? I would just like to get a sense for how important this is and if possibly there may be a workaround. @psonnera |
Whilst this change, if tested correctly, doesn't look bad at all, I believe the community would benefit having more information on the Nightscout deployment method and use case in question. Subpath probably allows reusing an existing DNS for either more apps or Nightscout instances. @jeroennijhof is that the case? |
@psonnera that's correct. We currently run 5 Nightscout instances and since we already had a ssl certificate, domain and vps it's pretty easy and cheap to run it all on one vm. Sure I can set it up with subdomains but that means I need to buy 5 ssl certificates to be more secure. I don't like to expose the api secrets on http. And since I run into this I figured there will be more people out there wanting to reuse their VM, DNS and SSL. @Navid200 thank you for your explanation I really understand the pain since I run a couple of o.s. projects myself so if you want to close the PR, that's fine. |
No, I don't want to close the PR. Thanks Let's now wait for his response. |
What are the pro's and cons of merging this? Can it break any current user's settings? |
I am embarrassed to say I jumped in and asked questions without looking at the literal changes. Just 2 slashes?! We just need to see test results. |
A single character is enough cause serious problems. This change maps the API to a relative instead of absolute root url. There is a very strong chance that it works but I would like to ensure the pros and cons have been tested to avoid us breaking any existing installations that might be affected by this. I would like to hear from @jeroennijhof if they think there could be an unintended consequences for existing users with this change? |
@jamorham the only way this change could have any unintended issues for people when they already have a subpath in the Nightscout url i.e. with the api full path. Then the url will become invalid as in /api/v1/api/v1/entries or sorts. But since the majority have Nightscout running under it's own domain name I think it's not a really big issue, but I'm not sure. |
@@ -212,4 +212,4 @@ public static void resetInstance() { | |||
UserError.Log.d(TAG, "Instance reset"); | |||
CollectionServiceStarter.restartCollectionServiceBackground(); | |||
} | |||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please do not remove the EOF line break!
Can this be detected and removed from the URL setting? A unit test would be great to test against this potential problem. Afterwards a fix can be implemented, i. e. by removing the API subpath. This is at least necessary for #1765. |
This will add support for subpaths in the Nightscout follower url.
Currently when you have a Nightscout instance running at i.e. https://domain.com/subpath/ it will strip the /subpath/ from the url which is not correct. By using relative paths for the @get methods subpaths are working.