Skip to content

Conversation

@jtc42
Copy link
Member

@jtc42 jtc42 commented Jun 22, 2020

Overhauls how schemas and args are defined in View classes. To end users of the library this will mean changing from:

@PropertySchema(fields.Int())
@Tag("properties")
@Doc(description="My cool class")
@ThingProperty
class MyAPIView(View):
   def get():
      return SomeObject

   def put(new_value):
      SomeObject = new_value

to

class MyAPIView(PropertyView):
   """My cool class"""

   schema = fields.Int()
   tags = ["properties"]

   def get():
      return SomeObject

   def put(new_value):
      SomeObject = new_value

In the back-end, this change has a few key advantages.

Firstly, defining a schema for marshalling responses no longer modifies the original get(), put() etc methods. Instead, marshalling happens in the dispatch_request function, and so happens at runtime. Likewise, request arguments are handled by dispatch_request, meaning that the original get(), put() etc methods can be called with Python arguments outside of any request context.

Furthermore, previously API spec information was stored in __apispec__ dictionaries on whatever object was decorated. This meant that view functions had this mysterious dunder property, which is bad practice in itself, but also the View class had to then step through each view method, parsing its
__apispec__ in order to build documentation.

Now, the schema, tags, args, title etc are defines as class attributes, with those names (see example above). The View class then uses these attributes to manipulate the request/response at request-time, meaning that we can build documentation just by looking at sensibly named attributes of the View class. We no longer have to insert bizarre dunder attributes into class method objects and such.

One aspect that will still use decorators is the semantic annotations. In this case, the semantic annotation will decorate the entire View class, and automatically apply schema, semtype and args attributes.

I think this makes the LabThings Python API much clearer, due to the reduced use of crazy nested decorators, and it's simplified the library code massively.

@codecov
Copy link

codecov bot commented Jun 23, 2020

Codecov Report

Merging #110 into master will decrease coverage by 0.41%.
The diff coverage is 88.83%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #110      +/-   ##
==========================================
- Coverage   94.88%   94.46%   -0.42%     
==========================================
  Files          40       41       +1     
  Lines        1779     1663     -116     
  Branches      275      255      -20     
==========================================
- Hits         1688     1571     -117     
+ Misses         63       54       -9     
- Partials       28       38      +10     
Flag Coverage Δ
#unittests 94.46% <88.83%> (-0.42%) ⬇️
Impacted Files Coverage Δ
src/labthings/core/lock.py 94.25% <ø> (+4.59%) ⬆️
src/labthings/server/default_views/root.py 80.00% <ø> (ø)
src/labthings/server/responses.py 0.00% <0.00%> (ø)
src/labthings/server/semantics/moz.py 100.00% <ø> (ø)
src/labthings/server/view/args.py 75.00% <75.00%> (ø)
src/labthings/server/view/marshalling.py 75.75% <75.75%> (ø)
src/labthings/server/labthing.py 98.44% <87.50%> (-0.54%) ⬇️
src/labthings/server/spec/apispec.py 93.10% <89.47%> (-3.68%) ⬇️
src/labthings/server/default_views/extensions.py 100.00% <100.00%> (ø)
src/labthings/server/default_views/sockets.py 87.50% <100.00%> (+23.86%) ⬆️
... and 12 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update b62879e...cc676b7. Read the comment docs.

@jtc42 jtc42 marked this pull request as ready for review June 30, 2020 13:51
@jtc42 jtc42 merged commit 3cf1559 into master Jun 30, 2020
@jtc42 jtc42 deleted the simpler-spec branch June 30, 2020 13:52
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants