Title: revisiting decorators for actions/properties/etc. · Issue #237 · labthings/python-labthings · GitHub
Open Graph Title: revisiting decorators for actions/properties/etc. · Issue #237 · labthings/python-labthings
X Title: revisiting decorators for actions/properties/etc. · Issue #237 · labthings/python-labthings
Description: I am sure we have discussed in the past the merits of defining a View subclass for each endpoint, e.g. in an Extension. I have a nasty feeling that I probably argued strongly against using decorators because they often make life quite co...
Open Graph Description: I am sure we have discussed in the past the merits of defining a View subclass for each endpoint, e.g. in an Extension. I have a nasty feeling that I probably argued strongly against using decorato...
X Description: I am sure we have discussed in the past the merits of defining a View subclass for each endpoint, e.g. in an Extension. I have a nasty feeling that I probably argued strongly against using decorato...
Opengraph URL: https://github.com/labthings/python-labthings/issues/237
X: @github
Domain: patch-diff.githubusercontent.com
{"@context":"https://schema.org","@type":"DiscussionForumPosting","headline":"revisiting decorators for actions/properties/etc.","articleBody":"I am sure we have discussed in the past the merits of defining a `View` subclass for each endpoint, e.g. in an `Extension`. I have a nasty feeling that I probably argued strongly against using decorators because they often make life quite confusing. However, I'm coming round to the idea. My use case is this:\r\n\r\nI'm writing some code in an extension, which includes a function to do something. For argument's sake let's say that function should be an `Action` but I think the same consideration applies to properties. Ideally, I'd like it to be accessible either via HTTP or via Python. Currently, what I do is:\r\n\r\n1. Write the Python function that does what I want (and give it a helpful docstring) as a method of my Extension\r\n2. Define another class wrapping that Python function in a way that can be called nicely from the API\r\n3. Reference said class from within the Extension class so it appears in the AP\r\n\r\nThere are several things that are less than ideal here:\r\n\r\n1. You end up defining your arguments in multiple places - specifically, in both the method and the View class.\r\n2. Docstrings don't propagate from the method to the View, so the HTTP API docs are not great\r\n\r\nI can see a few ways to make it better: \r\n1. Be more explicit about the fact that `ActionView` classes are more or less always wrapping a function to do an action. This would, for example, mean that instead of writing a `post` method, I would supply an `action` function. Doing that explicitly makes it way easier to wrap the action function nicely, e.g. propagate the docstrings.\r\n2. Provide a decorator to put the definition of the `ActionView` right next to the function it's wrapping. This makes it harder to miss things being out of sync, and could also eliminate the need to register views explicitly.\r\n\r\nI think if I argued against custom decorators in the past, it was because I felt they were non-standard ways of doing things, and wouldn't make sense to someone familiar with Flask. That's true, but I think there's enough that's non-standard about e.g. the way we generate our APISpec that this battle is possibly lost anyway. Also, if we define a more \"helpful\" `ActionView` subclass, it potentially provides a point of familiarity for Flask developers - so if they understand how `@action` maps onto `ActionViewWrapper` (which is potentially quite simple), it's easier to figure out what happens once you have a `View`.\r\n\r\nAlso, I think when we made this decision, it was more likely that you would want control over the get/post/delete/whatever methods. Now that the WoT specification is finalised, it's pretty well defined what each HTTP method should do, and I'm in favour of providing a mapping from HTTP methods to meaningful WoT interactions. I.e. a `PropertyView` always reads a property with `GET` and writes it with `PUT` and may define a way to update it with `POST`. So probably we should, by default, define a getter, setter, and update method - not all of which must exist. Similarly, an `ActionView` will always run actions with `POST` and return status with `GET`, so it makes sense to simply have a function responsible for running the action (which is what's currently implemented in `ActionView.post`. This also means that overriding `ActionView.post` would allow an action to do something custom *before* it's spawned in a background thread, such as validating arguments and returning an HTTP error code if they're wrong.\r\n\r\nI'm raising this issue so @jtc42 can tell me why I was dead against this in the past, or if he remembers anything important that's not reflected in the discussion above.","author":{"url":"https://github.com/rwb27","@type":"Person","name":"rwb27"},"datePublished":"2021-06-30T09:32:41.000Z","interactionStatistic":{"@type":"InteractionCounter","interactionType":"https://schema.org/CommentAction","userInteractionCount":3},"url":"https://github.com/237/python-labthings/issues/237"}
| route-pattern | /_view_fragments/issues/show/:user_id/:repository/:id/issue_layout(.:format) |
| route-controller | voltron_issues_fragments |
| route-action | issue_layout |
| fetch-nonce | v2:34807620-fafb-cb1d-d734-9429e74fa3ab |
| current-catalog-service-hash | 81bb79d38c15960b92d99bca9288a9108c7a47b18f2423d0f6438c5b7bcd2114 |
| request-id | E6D2:9763B:9893C5:C54375:698FD4F1 |
| html-safe-nonce | 7bef7a6da1ecf8896d73ca8f4a79ecd8f05d65e8e3f56b1913308520dfac3ba3 |
| visitor-payload | eyJyZWZlcnJlciI6IiIsInJlcXVlc3RfaWQiOiJFNkQyOjk3NjNCOjk4OTNDNTpDNTQzNzU6Njk4RkQ0RjEiLCJ2aXNpdG9yX2lkIjoiNjgxNzU4MzAwMTU0MDQ4MjI4OSIsInJlZ2lvbl9lZGdlIjoiaWFkIiwicmVnaW9uX3JlbmRlciI6ImlhZCJ9 |
| visitor-hmac | 5d1a6a2ff68704c02fee412ddd3d1e318a63a062f82d6453eabf8bfcabfad9b4 |
| hovercard-subject-tag | issue:933494361 |
| github-keyboard-shortcuts | repository,issues,copilot |
| google-site-verification | Apib7-x98H0j5cPqHWwSMm6dNU4GmODRoqxLiDzdx9I |
| octolytics-url | https://collector.github.com/github/collect |
| analytics-location | / |
| fb:app_id | 1401488693436528 |
| apple-itunes-app | app-id=1477376905, app-argument=https://github.com/_view_fragments/issues/show/labthings/python-labthings/237/issue_layout |
| twitter:image | https://opengraph.githubassets.com/b68fedd7e18504a8ff322ccd35cefdcc22f9734036f4d491cd88f9de7323d8d5/labthings/python-labthings/issues/237 |
| twitter:card | summary_large_image |
| og:image | https://opengraph.githubassets.com/b68fedd7e18504a8ff322ccd35cefdcc22f9734036f4d491cd88f9de7323d8d5/labthings/python-labthings/issues/237 |
| og:image:alt | I am sure we have discussed in the past the merits of defining a View subclass for each endpoint, e.g. in an Extension. I have a nasty feeling that I probably argued strongly against using decorato... |
| og:image:width | 1200 |
| og:image:height | 600 |
| og:site_name | GitHub |
| og:type | object |
| og:author:username | rwb27 |
| hostname | github.com |
| expected-hostname | github.com |
| None | 42c603b9d642c4a9065a51770f75e5e27132fef0e858607f5c9cb7e422831a7b |
| turbo-cache-control | no-preview |
| go-import | github.com/labthings/python-labthings git https://github.com/labthings/python-labthings.git |
| octolytics-dimension-user_id | 58913024 |
| octolytics-dimension-user_login | labthings |
| octolytics-dimension-repository_id | 234552142 |
| octolytics-dimension-repository_nwo | labthings/python-labthings |
| octolytics-dimension-repository_public | true |
| octolytics-dimension-repository_is_fork | false |
| octolytics-dimension-repository_network_root_id | 234552142 |
| octolytics-dimension-repository_network_root_nwo | labthings/python-labthings |
| turbo-body-classes | logged-out env-production page-responsive |
| disable-turbo | false |
| browser-stats-url | https://api.github.com/_private/browser/stats |
| browser-errors-url | https://api.github.com/_private/browser/errors |
| release | d320682233dfd4d28c0b30554a564c2fcd229032 |
| ui-target | full |
| theme-color | #1e2327 |
| color-scheme | light dark |
Links:
Viewport: width=device-width