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

Datasette on Zeit Now returns http URLs for facet and next links #333

Closed
simonw opened this issue Jul 6, 2018 · 4 comments
Closed

Datasette on Zeit Now returns http URLs for facet and next links #333

simonw opened this issue Jul 6, 2018 · 4 comments

Comments

@simonw
Copy link
Owner

simonw commented Jul 6, 2018

e.g. on https://fivethirtyeight.datasettes.com/fivethirtyeight-ac35616/nba-elo%2Fnbaallelo.json?_facet=lg_id&_size=0

{
  "facet_results": {
    "lg_id": {
      "name": "lg_id",
      "results": [
        {
          "value": "NBA",
          "label": "NBA",
          "count": 118016,
          "toggle_url": "http://fivethirtyeight.datasettes.com/fivethirtyeight-ac35616/nba-elo%2Fnbaallelo.json?_facet=lg_id&_size=1&lg_id=NBA",
          "selected": false
        },
        {
          "value": "ABA",
          "label": "ABA",
          "count": 8298,
          "toggle_url": "http://fivethirtyeight.datasettes.com/fivethirtyeight-ac35616/nba-elo%2Fnbaallelo.json?_facet=lg_id&_size=1&lg_id=ABA",
          "selected": false
        }
      ],
      "truncated": false
    }
  },
  "suggested_facets": [
    {
      "name": "_iscopy",
      "toggle_url": "/fivethirtyeight-ac35616/nba-elo%2Fnbaallelo.json?_facet=lg_id&_size=1&_facet=_iscopy"
    }
  ],
  "next_url": "http://fivethirtyeight.datasettes.com/fivethirtyeight-ac35616/nba-elo%2Fnbaallelo.json?_facet=lg_id&_size=1&_next=1",
}

next_url and facet_results both link to http:// when they should link to https://.

Note that suggested facets doesn't include the full URL at all, which is a consistency bug.

@simonw simonw added this to the Next release milestone Jul 10, 2018
@simonw
Copy link
Owner Author

simonw commented Jul 18, 2018

A force_https_api_urls config option would work here - if set, Datasette will ignore the incoming protocol and always use https. The datasette deploy now command could then add that as an option passed to datasette serve.

This is the pattern which is producing incorrect URLs on Zeit Now, because the Sanic request.url property is not being correctly set.

next_url = urllib.parse.urljoin(
request.url, path_with_replaced_args(request, added_args)
)

Suggested help text:

Always use https:// for URLs output as part of Datasette API responses

@simonw
Copy link
Owner Author

simonw commented Jul 18, 2018

I'll add a absolute_url(request, path) method on the base view class which knows to check the new config option.

simonw added a commit that referenced this issue Jul 23, 2018
@simonw
Copy link
Owner Author

simonw commented Jul 23, 2018

I still need to modify datasette publish now to set this config option on the instances that it deploys.

@simonw simonw closed this as completed in b320f58 Jul 24, 2018
@simonw
Copy link
Owner Author

simonw commented Jul 24, 2018

@simonw simonw removed this from the Next release milestone Jul 24, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

1 participant