Determine transfer protocol more precisely #14
Conversation
Current coverage is 77.90% (diff: 97.72%)@@ master #14 diff @@
==========================================
Files 35 35
Lines 4461 4521 +60
Methods 0 0
Messages 0 0
Branches 0 0
==========================================
+ Hits 3481 3522 +41
- Misses 980 999 +19
Partials 0 0
|
endpoints/api_config.py
Outdated
| protocol = ('http' if hostname and hostname.startswith('localhost') else | ||
| 'https') | ||
| else: | ||
| protocol = ('http' if endpoints_util.is_running_on_devserver() else |
There was a problem hiding this comment.
Why is the local param needed again? Why can't we just say http if on devserver, https otherwise?
There was a problem hiding this comment.
As I understand it, a config can be generated either remotely (via an HTTP call to the service) or locally (using endpointscfg). If it's done locally, then it doesn't make sense to check whether we're running on a dev_appserver or not; it's just being run from a command-line prompt.
Does that make sense?
There was a problem hiding this comment.
It seems to me that we don't really need to differentiate between the two cases. I feel like it should be possible to eliminate the local parameter and have the logic be:
if devserver or hostname == localhost, return HTTP, else HTTPS
endpointscfg.py supports overriding a hostname so it should be enough to predicate HTTP on those two things.
|
Alright, per our discussion offline, this is quite a bit simpler. :) PTAL |
endpoints/swagger_generator.py
Outdated
| hostname = (hostname or util.get_app_hostname() or | ||
| api_info.hostname) | ||
| protocol = ('http' if hostname and hostname.startswith('localhost') else | ||
| 'https') |
There was a problem hiding this comment.
Should this also be updated?
There was a problem hiding this comment.
Yep. I also wrote some new tests for swagger generation to make sure the protocol is showing up correctly in the devserver case there, along with some refactoring for the test. PTAL.
3 participants
If the config is being generated locally (using endpointscfg.py), we stick to the old logic of "HTTP if localhost else HTTPS". When it's being generated remotely (using apiserving.py), we use the new logic: "HTTP if running on development server, else HTTPS"