issue_comments
12 rows where issue = 463544206
This data as json, CSV (advanced)
Suggested facets: user, author_association, reactions, 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 |
|---|---|---|---|---|---|---|---|---|---|---|---|
| 512126748 | https://github.com/simonw/datasette/issues/537#issuecomment-512126748 | https://api.github.com/repos/simonw/datasette/issues/537 | MDEyOklzc3VlQ29tbWVudDUxMjEyNjc0OA== | SteadBytes 14834132 | 2019-07-17T06:48:35Z | 2019-07-17T06:48:35Z | NONE | It looks as if the `datasette.utils.AsgiRouter.__call__` is the place to add this https://github.com/simonw/datasette/blob/90d4f497f9b3f6a5882937c91fddb496ac3e7368/datasette/utils/asgi.py#L101 . The sentry_asgi middleware uses the `__qualname__` or `__name__` attributes of the `endpoint` https://github.com/encode/sentry-asgi/blob/c6a42d44d31f85885b79e4ee898683ecf8104971/sentry_asgi/middleware.py#L84 Looking at the Starlette implementation `endpoint` is a `Callable` https://github.com/encode/starlette/commit/34d0097feb6f057bd050d5057df5a2f96b97384e#diff-34fba745b50527bfb4245d02afd59246R100 which as far as I can tell is analogous to the `view` function which is matched here https://github.com/simonw/datasette/blob/90d4f497f9b3f6a5882937c91fddb496ac3e7368/datasette/utils/asgi.py#L96 . A slight issue is that `__qualname__` is matched *first* in the sentry_asgi middleware, and `__name__` is used if that doesn't exist. I think (please correct me if I am wrong) that for datasette, the `__name__` is what should be used. For example, when using the development fixtures and hitting `http://127.0.0.1:8001/fixtures/compound_three_primary_keys` the `view` function that is matched gives: ```python >>> view.__qualname__ 'AsgiView.as_asgi.<locals>.view' >>> view.__name__ 'TableView' ``` Would `TableView` be the desired value here? Or am I looking in entirely the wrong place? :smile: | {"total_count": 0, "+1": 0, "-1": 0, "laugh": 0, "hooray": 0, "confused": 0, "heart": 0, "rocket": 0, "eyes": 0} | Populate "endpoint" key in ASGI scope 463544206 | |
| 512664216 | https://github.com/simonw/datasette/issues/537#issuecomment-512664216 | https://api.github.com/repos/simonw/datasette/issues/537 | MDEyOklzc3VlQ29tbWVudDUxMjY2NDIxNg== | simonw 9599 | 2019-07-18T04:53:18Z | 2019-07-18T04:53:18Z | OWNER | Yes, `TableView` is desirable here I think. Maybe `datasette.views.table.TableView` if we can get that somehow. Thanks for taking a look! | {"total_count": 0, "+1": 0, "-1": 0, "laugh": 0, "hooray": 0, "confused": 0, "heart": 0, "rocket": 0, "eyes": 0} | Populate "endpoint" key in ASGI scope 463544206 | |
| 512930353 | https://github.com/simonw/datasette/issues/537#issuecomment-512930353 | https://api.github.com/repos/simonw/datasette/issues/537 | MDEyOklzc3VlQ29tbWVudDUxMjkzMDM1Mw== | SteadBytes 14834132 | 2019-07-18T18:20:53Z | 2019-07-18T18:34:03Z | NONE | Ok great, getting the `__qualname__` to be `TableView` and adding `endpoint` to the `scope` in `AsgiRouter` is simple enough (already done). However, (unless I'm missing a plugin hook or something) the suggestion of utilising it within a `datasette-sentry` plugin may not work. The only hook that would have access to the `scope` is the `asgi_wrapper` hook. But as this _wraps_ the existing `asgi` app, the `endpoint` won't yet have been added to the `scope` received by the hook https://github.com/SteadBytes/datasette/blob/107d47567dedd472eebec7f35bc34f5b58285ba8/datasette/app.py#L672 . However, I'm not sure where else the `endpoint` could be added to the asgi scope :thinking: | {"total_count": 0, "+1": 0, "-1": 0, "laugh": 0, "hooray": 0, "confused": 0, "heart": 0, "rocket": 0, "eyes": 0} | Populate "endpoint" key in ASGI scope 463544206 | |
| 513272392 | https://github.com/simonw/datasette/issues/537#issuecomment-513272392 | https://api.github.com/repos/simonw/datasette/issues/537 | MDEyOklzc3VlQ29tbWVudDUxMzI3MjM5Mg== | simonw 9599 | 2019-07-19T15:27:03Z | 2019-07-19T15:27:03Z | OWNER | Yeah that's a good call: the Datasette plugin mechanism where middleware is wrapped around the outside doesn't appear to be compatible with the Sentry mechanism of expecting that `scope` has been populated before it gets to their error handler. @tomchristie is this something you've thought about? | {"total_count": 0, "+1": 0, "-1": 0, "laugh": 0, "hooray": 0, "confused": 0, "heart": 0, "rocket": 0, "eyes": 0} | Populate "endpoint" key in ASGI scope 463544206 | |
| 513273003 | https://github.com/simonw/datasette/issues/537#issuecomment-513273003 | https://api.github.com/repos/simonw/datasette/issues/537 | MDEyOklzc3VlQ29tbWVudDUxMzI3MzAwMw== | simonw 9599 | 2019-07-19T15:28:42Z | 2019-07-19T15:28:42Z | OWNER | Asked about this on Twitter: https://twitter.com/simonw/status/1152238730259791877 | {"total_count": 1, "+1": 0, "-1": 0, "laugh": 1, "hooray": 0, "confused": 0, "heart": 0, "rocket": 0, "eyes": 0} | Populate "endpoint" key in ASGI scope 463544206 | |
| 513279397 | https://github.com/simonw/datasette/issues/537#issuecomment-513279397 | https://api.github.com/repos/simonw/datasette/issues/537 | MDEyOklzc3VlQ29tbWVudDUxMzI3OTM5Nw== | tomchristie 647359 | 2019-07-19T15:47:57Z | 2019-07-19T15:48:09Z | NONE | The middleware implementation there works okay with a router nested inside if the scope is *mutated*. (Ie. "endpoint" doesn't need to exist at the point that the middleware starts running, but if it *has* been made available by the time an exception is thrown, then it can be used.) Starlette's usage of "endpoint" there is unilateral, rather than something I've discussed against the ASGI spec - certainly it's important for any monitoring ASGI middleware to be able to have some kind of visibility onto some limited subset of routing information, and `"endpoint"` in the scope referencing some routed-to callable seemed general enough to be useful. | {"total_count": 0, "+1": 0, "-1": 0, "laugh": 0, "hooray": 0, "confused": 0, "heart": 0, "rocket": 0, "eyes": 0} | Populate "endpoint" key in ASGI scope 463544206 | |
| 513307487 | https://github.com/simonw/datasette/issues/537#issuecomment-513307487 | https://api.github.com/repos/simonw/datasette/issues/537 | MDEyOklzc3VlQ29tbWVudDUxMzMwNzQ4Nw== | simonw 9599 | 2019-07-19T17:17:43Z | 2019-07-19T17:17:43Z | OWNER | Huh, interesting. I'd got it into my head that scope should not be mutated under any circumstances - if that's not true and it's mutable there's all kinds of useful things we could do with it. | {"total_count": 0, "+1": 0, "-1": 0, "laugh": 0, "hooray": 0, "confused": 0, "heart": 0, "rocket": 0, "eyes": 0} | Populate "endpoint" key in ASGI scope 463544206 | |
| 513317952 | https://github.com/simonw/datasette/issues/537#issuecomment-513317952 | https://api.github.com/repos/simonw/datasette/issues/537 | MDEyOklzc3VlQ29tbWVudDUxMzMxNzk1Mg== | simonw 9599 | 2019-07-19T17:49:06Z | 2019-07-19T17:49:06Z | OWNER | It strikes me that if scope is indeed meant to stay immutable the alternative way of solving this would be to add an outbound custom request header with the endpoint - `X-Endpoint: datasette.views.table.TableView` for example - and teach the Sentry plugin to optionally read that. | {"total_count": 0, "+1": 0, "-1": 0, "laugh": 0, "hooray": 0, "confused": 0, "heart": 0, "rocket": 0, "eyes": 0} | Populate "endpoint" key in ASGI scope 463544206 | |
| 513439736 | https://github.com/simonw/datasette/issues/537#issuecomment-513439736 | https://api.github.com/repos/simonw/datasette/issues/537 | MDEyOklzc3VlQ29tbWVudDUxMzQzOTczNg== | SteadBytes 14834132 | 2019-07-20T06:05:01Z | 2019-07-20T06:05:01Z | NONE | The asgi spec doesn't explicitly specify (at least as far as I can tell) whether the scope is immutable/mutable https://asgi.readthedocs.io/en/latest/specs/lifespan.html#scope . @simonw using a header for this would be a nice approach. It would also potentially increase the portability of any middleware/plugins/clients across different applications/frameworks as it's not tied directly to an asgi implementation | {"total_count": 0, "+1": 0, "-1": 0, "laugh": 0, "hooray": 0, "confused": 0, "heart": 0, "rocket": 0, "eyes": 0} | Populate "endpoint" key in ASGI scope 463544206 | |
| 513442743 | https://github.com/simonw/datasette/issues/537#issuecomment-513442743 | https://api.github.com/repos/simonw/datasette/issues/537 | MDEyOklzc3VlQ29tbWVudDUxMzQ0Mjc0Mw== | tomchristie 647359 | 2019-07-20T06:50:47Z | 2019-07-20T06:50:47Z | NONE | Right now the spec does say “copy the scope, rather than mutate it” https://asgi.readthedocs.io/en/latest/specs/main.html#middleware I wouldn’t be surprised if that there’s room for discussion on evolving the exact language there. There’s obvs a nice element to the strictness there, tho practically I’m not sure it’s something that implementations will follow, and its not something that Starlette chooses to abide by. | {"total_count": 0, "+1": 0, "-1": 0, "laugh": 0, "hooray": 0, "confused": 0, "heart": 0, "rocket": 0, "eyes": 0} | Populate "endpoint" key in ASGI scope 463544206 | |
| 513446227 | https://github.com/simonw/datasette/issues/537#issuecomment-513446227 | https://api.github.com/repos/simonw/datasette/issues/537 | MDEyOklzc3VlQ29tbWVudDUxMzQ0NjIyNw== | SteadBytes 14834132 | 2019-07-20T07:50:44Z | 2019-07-20T07:50:44Z | NONE | Oh yes well spotted thank you 😁 I agree that the strictness would be nice as it could help to avoid different middleware altering the scope in incompatible ways. However I do also agree that it's likely for not all implementations to follow 🤔 | {"total_count": 0, "+1": 0, "-1": 0, "laugh": 0, "hooray": 0, "confused": 0, "heart": 0, "rocket": 0, "eyes": 0} | Populate "endpoint" key in ASGI scope 463544206 | |
| 513652597 | https://github.com/simonw/datasette/issues/537#issuecomment-513652597 | https://api.github.com/repos/simonw/datasette/issues/537 | MDEyOklzc3VlQ29tbWVudDUxMzY1MjU5Nw== | SteadBytes 14834132 | 2019-07-22T06:03:18Z | 2019-07-22T06:03:18Z | NONE | @simonw do you think it is still worth populating the `endpoint` key in the scope as originally intended by this issue, or should we hold off until a decision about possibly using an `X-Endpoint` header instead? :smile: | {"total_count": 0, "+1": 0, "-1": 0, "laugh": 0, "hooray": 0, "confused": 0, "heart": 0, "rocket": 0, "eyes": 0} | Populate "endpoint" key in ASGI scope 463544206 |
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]);