issue_comments
22 rows where issue = 1058790545
This data as json, CSV (advanced)
Suggested facets: user, author_association, created_at (date), updated_at (date)
| id ▼ | html_url | issue_url | node_id | user | created_at | updated_at | author_association | body | reactions | issue | performed_via_github_app |
|---|---|---|---|---|---|---|---|---|---|---|---|
| 974309591 | https://github.com/simonw/datasette/issues/1519#issuecomment-974309591 | https://api.github.com/repos/simonw/datasette/issues/1519 | IC_kwDOBm6k_c46EsjX | simonw 9599 | 2021-11-19T18:31:32Z | 2021-11-19T18:31:32Z | OWNER | `base_url` has been a source of so many bugs like this! I often find them quite hard to replicate, likely because I haven't made myself a good Apache `mod_proxy` testing environment yet. | {"total_count": 0, "+1": 0, "-1": 0, "laugh": 0, "hooray": 0, "confused": 0, "heart": 0, "rocket": 0, "eyes": 0} | base_url is omitted in JSON and CSV views 1058790545 | |
| 974310208 | https://github.com/simonw/datasette/issues/1519#issuecomment-974310208 | https://api.github.com/repos/simonw/datasette/issues/1519 | IC_kwDOBm6k_c46EstA | simonw 9599 | 2021-11-19T18:32:31Z | 2021-11-19T18:32:31Z | OWNER | Having a live demo running on Cloud Run that proxies through Apache and uses `base_url` would be incredibly useful for replicating and debugging this kind of thing. I wonder how hard it is to run Apache and `mod_proxy` in the same Docker container as Datasette? | {"total_count": 0, "+1": 0, "-1": 0, "laugh": 0, "hooray": 0, "confused": 0, "heart": 0, "rocket": 0, "eyes": 0} | base_url is omitted in JSON and CSV views 1058790545 | |
| 974389472 | https://github.com/simonw/datasette/issues/1519#issuecomment-974389472 | https://api.github.com/repos/simonw/datasette/issues/1519 | IC_kwDOBm6k_c46FADg | simonw 9599 | 2021-11-19T20:01:02Z | 2021-11-19T20:01:02Z | OWNER | I now have a `Dockerfile` in https://github.com/simonw/datasette/issues/1521#issuecomment-974388295 that I can use to run a local Apache 2 with `mod_proxy` to investigate this class of bugs! | {"total_count": 0, "+1": 0, "-1": 0, "laugh": 0, "hooray": 0, "confused": 0, "heart": 0, "rocket": 0, "eyes": 0} | base_url is omitted in JSON and CSV views 1058790545 | |
| 974391204 | https://github.com/simonw/datasette/issues/1519#issuecomment-974391204 | https://api.github.com/repos/simonw/datasette/issues/1519 | IC_kwDOBm6k_c46FAek | simonw 9599 | 2021-11-19T20:02:41Z | 2021-11-19T20:02:41Z | OWNER | Bug confirmed:  | {"total_count": 0, "+1": 0, "-1": 0, "laugh": 0, "hooray": 0, "confused": 0, "heart": 0, "rocket": 0, "eyes": 0} | base_url is omitted in JSON and CSV views 1058790545 | |
| 974398399 | https://github.com/simonw/datasette/issues/1519#issuecomment-974398399 | https://api.github.com/repos/simonw/datasette/issues/1519 | IC_kwDOBm6k_c46FCO_ | simonw 9599 | 2021-11-19T20:08:20Z | 2021-11-19T20:22:02Z | OWNER | The relevant test is this one: https://github.com/simonw/datasette/blob/30255055150d7bc0affc8156adc18295495020ff/tests/test_html.py#L1608-L1649 I modified that test to add `"/fixtures/facetable?sql=select+1"` as one of the tested paths, and dropped in an `assert False` to pause it in the debugger: ``` @pytest.mark.parametrize( "path", [ "/", "/fixtures", "/fixtures/compound_three_primary_keys", "/fixtures/compound_three_primary_keys/a,a,a", "/fixtures/paginated_view", "/fixtures/facetable", "/fixtures?sql=select+1", ], ) def test_base_url_config(app_client_base_url_prefix, path): client = app_client_base_url_prefix response = client.get("/prefix/" + path.lstrip("/")) soup = Soup(response.body, "html.parser") if path == "/fixtures?sql=select+1": > assert False E assert False ``` BUT... in the debugger: ``` (Pdb) print(soup) ... <p class="export-links">This data as <a href="/prefix/fixtures.json?sql=select+1">json</a>, <a href="/prefix/fixtures.testall?sql=select+1">testall</a>, <a href="/prefix/fixtures.testnone?sql=select+1">testnone</a>, <a href="/prefix/fixtures.testresponse?sql=select+1">testresponse</a>, <a href="/prefix/fixtures.csv?sql=select+1&_size=max">CSV</a></p> ``` Those all have the correct prefix! But that's not what I'm seeing in my `Dockerfile` reproduction of the issue. Something very weird is going on here. | {"total_count": 0, "+1": 0, "-1": 0, "laugh": 0, "hooray": 0, "confused": 0, "heart": 0, "rocket": 0, "eyes": 0} | base_url is omitted in JSON and CSV views 1058790545 | |
| 974405016 | https://github.com/simonw/datasette/issues/1519#issuecomment-974405016 | https://api.github.com/repos/simonw/datasette/issues/1519 | IC_kwDOBm6k_c46FD2Y | simonw 9599 | 2021-11-19T20:14:19Z | 2021-11-19T20:15:05Z | OWNER | I added `template_debug` in the Dockerfile: ``` datasette fixtures.db --setting template_debug 1 --setting base_url "/foo/bar/" -p 9000 &\n\ ``` And then hit `http://localhost:5000/foo/bar/fixtures?sql=select+*+from+compound_three_primary_keys+limit+1&_context=1` to view the template context - and it showed the bug, output edited to just show relevant keys: ```json { "edit_sql_url": "/foo/bar/fixtures?sql=select+%2A+from+compound_three_primary_keys+limit+1", "settings": { "force_https_urls": false, "template_debug": true, "trace_debug": false, "base_url": "/foo/bar/" }, "show_hide_link": "/fixtures?sql=select+%2A+from+compound_three_primary_keys+limit+1&_context=1&_hide_sql=1", "show_hide_text": "hide", "show_hide_hidden": "", "renderers": { "json": "/fixtures.json?sql=select+*+from+compound_three_primary_keys+limit+1&_context=1" }, "url_csv": "/fixtures.csv?sql=select+*+from+compound_three_primary_keys+limit+1&_context=1&_size=max", "url_csv_path": "/fixtures.csv", "base_url": "/foo/bar/" } ``` This is so strange. `edit_sql_url` and `base_url` are correct, but `show_hide_link` and `url_csv` and `renderers.json` are not. And it's _really strange_ that the bug doesn't show up in the tests. | {"total_count": 0, "+1": 0, "-1": 0, "laugh": 0, "hooray": 0, "confused": 0, "heart": 0, "rocket": 0, "eyes": 0} | base_url is omitted in JSON and CSV views 1058790545 | |
| 974418496 | https://github.com/simonw/datasette/issues/1519#issuecomment-974418496 | https://api.github.com/repos/simonw/datasette/issues/1519 | IC_kwDOBm6k_c46FHJA | simonw 9599 | 2021-11-19T20:24:16Z | 2021-11-19T20:24:16Z | OWNER | Here's the code that generates `edit_sql_url` correctly: https://github.com/simonw/datasette/blob/85849935292e500ab7a99f8fe0f9546e903baad3/datasette/views/database.py#L416-L420 And here's the code for `show_hide_link`: https://github.com/simonw/datasette/blob/85849935292e500ab7a99f8fe0f9546e903baad3/datasette/views/database.py#L432-L433 And for `url_csv`: https://github.com/simonw/datasette/blob/85849935292e500ab7a99f8fe0f9546e903baad3/datasette/views/base.py#L600-L602 | {"total_count": 0, "+1": 0, "-1": 0, "laugh": 0, "hooray": 0, "confused": 0, "heart": 0, "rocket": 0, "eyes": 0} | base_url is omitted in JSON and CSV views 1058790545 | |
| 974420619 | https://github.com/simonw/datasette/issues/1519#issuecomment-974420619 | https://api.github.com/repos/simonw/datasette/issues/1519 | IC_kwDOBm6k_c46FHqL | simonw 9599 | 2021-11-19T20:25:19Z | 2021-11-19T20:25:19Z | OWNER | The implementations of `path_with_removed_args` and `path_with_format`: https://github.com/simonw/datasette/blob/85849935292e500ab7a99f8fe0f9546e903baad3/datasette/utils/__init__.py#L228-L254 https://github.com/simonw/datasette/blob/85849935292e500ab7a99f8fe0f9546e903baad3/datasette/utils/__init__.py#L710-L729 | {"total_count": 0, "+1": 0, "-1": 0, "laugh": 0, "hooray": 0, "confused": 0, "heart": 0, "rocket": 0, "eyes": 0} | base_url is omitted in JSON and CSV views 1058790545 | |
| 974422829 | https://github.com/simonw/datasette/issues/1519#issuecomment-974422829 | https://api.github.com/repos/simonw/datasette/issues/1519 | IC_kwDOBm6k_c46FIMt | simonw 9599 | 2021-11-19T20:26:35Z | 2021-11-19T20:26:35Z | OWNER | In the `?_context=` debug view the request looks like this: ``` "request": "<datasette.utils.asgi.Request object at 0x7faf9fe06200>", ``` I'm going to add a `repr()` to it such that it's a bit more useful. | {"total_count": 0, "+1": 0, "-1": 0, "laugh": 0, "hooray": 0, "confused": 0, "heart": 0, "rocket": 0, "eyes": 0} | base_url is omitted in JSON and CSV views 1058790545 | |
| 974433206 | https://github.com/simonw/datasette/issues/1519#issuecomment-974433206 | https://api.github.com/repos/simonw/datasette/issues/1519 | IC_kwDOBm6k_c46FKu2 | simonw 9599 | 2021-11-19T20:31:52Z | 2021-11-19T20:31:52Z | OWNER | Modified my `Dockerfile` to do this: RUN pip install https://github.com/simonw/datasette/archive/ff0dd4da38d48c2fa9250ecf336002c9ed724e36.zip And now the `request` in that debug `?_context=1` looks like this: ``` "request": "<asgi.Request method=\"GET\" url=\"http://localhost:9000/fixtures?sql=select+*+from+compound_three_primary_keys+limit+1&_context=1\">" ``` That explains the bug - that request doesn't maintain the original path prefix of `http://localhost:5000/foo/bar/fixtures?sql=` (also it's been rewritten to `localhost:9000` instead of `localhost:5000`). | {"total_count": 0, "+1": 0, "-1": 0, "laugh": 0, "hooray": 0, "confused": 0, "heart": 0, "rocket": 0, "eyes": 0} | base_url is omitted in JSON and CSV views 1058790545 | |
| 974433320 | https://github.com/simonw/datasette/issues/1519#issuecomment-974433320 | https://api.github.com/repos/simonw/datasette/issues/1519 | IC_kwDOBm6k_c46FKwo | simonw 9599 | 2021-11-19T20:32:04Z | 2021-11-19T20:32:04Z | OWNER | Still not clear why the tests pass but the live example fails. | {"total_count": 0, "+1": 0, "-1": 0, "laugh": 0, "hooray": 0, "confused": 0, "heart": 0, "rocket": 0, "eyes": 0} | base_url is omitted in JSON and CSV views 1058790545 | |
| 974447950 | https://github.com/simonw/datasette/issues/1519#issuecomment-974447950 | https://api.github.com/repos/simonw/datasette/issues/1519 | IC_kwDOBm6k_c46FOVO | simonw 9599 | 2021-11-19T20:40:19Z | 2021-11-19T20:40:19Z | OWNER | Figured it out! The test is not an accurate recreation of what is happening, because it doesn't simulate a request with a path of `/fixtures` that has been redirected by the proxy to `/prefix/fixtures`. | {"total_count": 0, "+1": 0, "-1": 0, "laugh": 0, "hooray": 0, "confused": 0, "heart": 0, "rocket": 0, "eyes": 0} | base_url is omitted in JSON and CSV views 1058790545 | |
| 974450232 | https://github.com/simonw/datasette/issues/1519#issuecomment-974450232 | https://api.github.com/repos/simonw/datasette/issues/1519 | IC_kwDOBm6k_c46FO44 | simonw 9599 | 2021-11-19T20:41:53Z | 2021-11-19T20:42:19Z | OWNER | https://docs.datasette.io/en/stable/deploying.html#apache-proxy-configuration says I should use `ProxyPreserveHost on`. | {"total_count": 0, "+1": 0, "-1": 0, "laugh": 0, "hooray": 0, "confused": 0, "heart": 0, "rocket": 0, "eyes": 0} | base_url is omitted in JSON and CSV views 1058790545 | |
| 974477465 | https://github.com/simonw/datasette/issues/1519#issuecomment-974477465 | https://api.github.com/repos/simonw/datasette/issues/1519 | IC_kwDOBm6k_c46FViZ | simonw 9599 | 2021-11-19T21:15:30Z | 2021-11-19T21:15:30Z | OWNER | I think what's happening here is Apache is actually making a request to `/fixtures` rather than making a request to `/prefix/fixtures` - and Datasette is replying to requests on both the prefixed and the non-prefixed paths. This is pretty confusing! I think Datasette should ONLY reply to `/prefix/fixtures` instead and return a 404 for `/fixtures` - this would make things a whole lot easier to debug. But shipping that change could break existing deployments. Maybe that should be a breaking change for 1.0. | {"total_count": 0, "+1": 0, "-1": 0, "laugh": 0, "hooray": 0, "confused": 0, "heart": 0, "rocket": 0, "eyes": 0} | base_url is omitted in JSON and CSV views 1058790545 | |
| 974478126 | https://github.com/simonw/datasette/issues/1519#issuecomment-974478126 | https://api.github.com/repos/simonw/datasette/issues/1519 | IC_kwDOBm6k_c46FVsu | simonw 9599 | 2021-11-19T21:16:36Z | 2021-11-19T21:16:36Z | OWNER | In the meantime I can catch these errors by changing the test to run each path twice, once with and once without the prefix. This should accurately simulate how Apache is working here. | {"total_count": 0, "+1": 0, "-1": 0, "laugh": 0, "hooray": 0, "confused": 0, "heart": 0, "rocket": 0, "eyes": 0} | base_url is omitted in JSON and CSV views 1058790545 | |
| 974558267 | https://github.com/simonw/datasette/issues/1519#issuecomment-974558267 | https://api.github.com/repos/simonw/datasette/issues/1519 | IC_kwDOBm6k_c46FpQ7 | simonw 9599 | 2021-11-20T00:37:57Z | 2021-11-20T00:37:57Z | OWNER | Thanks to #1522 I have a live demo that exhibits this bug now: https://apache-proxy-demo.datasette.io/prefix/fixtures/attraction_characteristic | {"total_count": 0, "+1": 0, "-1": 0, "laugh": 0, "hooray": 0, "confused": 0, "heart": 0, "rocket": 0, "eyes": 0} | base_url is omitted in JSON and CSV views 1058790545 | |
| 974559176 | https://github.com/simonw/datasette/issues/1519#issuecomment-974559176 | https://api.github.com/repos/simonw/datasette/issues/1519 | IC_kwDOBm6k_c46FpfI | simonw 9599 | 2021-11-20T00:42:08Z | 2021-11-20T00:42:08Z | OWNER | > In the meantime I can catch these errors by changing the test to run each path twice, once with and once without the prefix. This should accurately simulate how Apache is working here. This worked, I managed to get the tests to fail! Here's the change I made: ```diff diff --git a/tests/test_html.py b/tests/test_html.py index f24165b..dbdfe59 100644 --- a/tests/test_html.py +++ b/tests/test_html.py @@ -1614,12 +1614,19 @@ def test_metadata_sort_desc(app_client): "/fixtures/compound_three_primary_keys/a,a,a", "/fixtures/paginated_view", "/fixtures/facetable", + "/fixtures?sql=select+1", ], ) -def test_base_url_config(app_client_base_url_prefix, path): +@pytest.mark.parametrize("use_prefix", (True, False)) +def test_base_url_config(app_client_base_url_prefix, path, use_prefix): client = app_client_base_url_prefix - response = client.get("/prefix/" + path.lstrip("/")) + path_to_get = path + if use_prefix: + path_to_get = "/prefix/" + path.lstrip("/") + response = client.get(path_to_get) soup = Soup(response.body, "html.parser") + if path == "/fixtures?sql=select+1": + assert False for el in soup.findAll(["a", "link", "script"]): if "href" in el.attrs: href = el["href"] @@ -1642,11 +1649,12 @@ def test_base_url_config(app_client_base_url_prefix, path): # If this has been made absolute it may start http://localhost/ if href.startswith("http://localhost/"): href = href[len("http://localost/") :] - assert href.startswith("/prefix/"), { + assert href.startswith("/prefix/"), json.dumps({ "path": path, + "path_to_get": path_to_get, "href_or_src": href, "element_parent": str(el.parent), - } + }, indent=4, default=repr) def test_base_url_affects_metadata_extra_css_urls(app_client_base_url_prefix): ``` | {"total_count": 0, "+1": 0, "-1": 0, "laugh": 0, "hooray": 0, "confused": 0, "heart": 0, "rocket": 0, "eyes": 0} | base_url is omitted in JSON and CSV views 1058790545 | |
| 974561593 | https://github.com/simonw/datasette/issues/1519#issuecomment-974561593 | https://api.github.com/repos/simonw/datasette/issues/1519 | IC_kwDOBm6k_c46FqE5 | simonw 9599 | 2021-11-20T00:53:19Z | 2021-11-20T00:53:19Z | OWNER | Adding that test found (I hope!) all of the remaining `base_url` bugs. There were a bunch! I think I finally get to close #838 too. | {"total_count": 0, "+1": 0, "-1": 0, "laugh": 0, "hooray": 0, "confused": 0, "heart": 0, "rocket": 0, "eyes": 0} | base_url is omitted in JSON and CSV views 1058790545 | |
| 974562942 | https://github.com/simonw/datasette/issues/1519#issuecomment-974562942 | https://api.github.com/repos/simonw/datasette/issues/1519 | IC_kwDOBm6k_c46FqZ- | simonw 9599 | 2021-11-20T00:59:32Z | 2021-11-20T00:59:32Z | OWNER | Ouch a nasty bug crept through there - https://datasette-apache-proxy-demo-j7hipcg4aq-uc.a.run.app/prefix/fixtures/compound_three_primary_keys says > 500: name 'ds' is not defined | {"total_count": 0, "+1": 0, "-1": 0, "laugh": 0, "hooray": 0, "confused": 0, "heart": 0, "rocket": 0, "eyes": 0} | base_url is omitted in JSON and CSV views 1058790545 | |
| 974697824 | https://github.com/simonw/datasette/issues/1519#issuecomment-974697824 | https://api.github.com/repos/simonw/datasette/issues/1519 | IC_kwDOBm6k_c46GLVg | simonw 9599 | 2021-11-20T19:11:21Z | 2021-11-20T19:11:21Z | OWNER | OK, i think I got all of them this time! The latest demo is now live at https://datasette-apache-proxy-demo.fly.dev/prefix/fixtures/sortable?_facet=pk2 I'm closing this issue, but feel free to re-open it if you spot any that I missed. | {"total_count": 0, "+1": 0, "-1": 0, "laugh": 0, "hooray": 0, "confused": 0, "heart": 0, "rocket": 0, "eyes": 0} | base_url is omitted in JSON and CSV views 1058790545 | |
| 974701788 | https://github.com/simonw/datasette/issues/1519#issuecomment-974701788 | https://api.github.com/repos/simonw/datasette/issues/1519 | IC_kwDOBm6k_c46GMTc | simonw 9599 | 2021-11-20T19:42:29Z | 2021-11-20T19:42:29Z | OWNER | > I think what's happening here is Apache is actually making a request to `/fixtures` rather than making a request to `/prefix/fixtures` - and Datasette is replying to requests on both the prefixed and the non-prefixed paths. > > This is pretty confusing! I think Datasette should ONLY reply to `/prefix/fixtures` instead and return a 404 for `/fixtures` - this would make things a whole lot easier to debug. > > But shipping that change could break existing deployments. Maybe that should be a breaking change for 1.0. On further thought I'm not going to do this. Having Datasette work behind a proxy the way it does right now is clearly easy for people to deploy (now that I've fixed the bugs) and I trust my improved tests to catch problems in the future. | {"total_count": 0, "+1": 0, "-1": 0, "laugh": 0, "hooray": 0, "confused": 0, "heart": 0, "rocket": 0, "eyes": 0} | base_url is omitted in JSON and CSV views 1058790545 | |
| 983890815 | https://github.com/simonw/datasette/issues/1519#issuecomment-983890815 | https://api.github.com/repos/simonw/datasette/issues/1519 | IC_kwDOBm6k_c46pPt_ | phubbard 157158 | 2021-12-01T17:50:09Z | 2021-12-01T17:50:09Z | NONE | thanks so very much for the prompt attention and fix! Plus, the animated GIF showing the bug is just extra and I love it. Interactions like this are why I love open source. | {"total_count": 0, "+1": 0, "-1": 0, "laugh": 0, "hooray": 0, "confused": 0, "heart": 0, "rocket": 0, "eyes": 0} | base_url is omitted in JSON and CSV views 1058790545 |
Advanced export
JSON shape: default, array, newline-delimited, object
CREATE TABLE [issue_comments] (
[html_url] TEXT,
[issue_url] TEXT,
[id] INTEGER PRIMARY KEY,
[node_id] TEXT,
[user] INTEGER REFERENCES [users]([id]),
[created_at] TEXT,
[updated_at] TEXT,
[author_association] TEXT,
[body] TEXT,
[reactions] TEXT,
[issue] INTEGER REFERENCES [issues]([id])
, [performed_via_github_app] TEXT);
CREATE INDEX [idx_issue_comments_issue]
ON [issue_comments] ([issue]);
CREATE INDEX [idx_issue_comments_user]
ON [issue_comments] ([user]);