First draft to show how to make syntactically nicer with type annotations

[ghandic]

Just the initial example - dont want to go too far down the rabbit hole if there's no appetite. Open to thoughts

Example

class HelloWorldService(ServiceBase):
    @typed_rpc
    def say_hello(
        self,
        name: Unicode,
        times: UnsignedInteger32,
        a: Iterable(Decimal),
        b: Iterable(Iterable(Decimal)),
    ) -> Iterable(Unicode):
        for _ in range(times):
            yield f"Hello, {name}"

Example 2

class HelloWorldService(ServiceBase):
    @typed_rpc(_is_async=True)
    def say_hello(
        self,
        name: Unicode,
        times: UnsignedInteger32,
        a: Iterable(Decimal),
        b: Iterable(Iterable(Decimal)),
    ) -> Iterable(Unicode):
        for _ in range(times):
            yield f"Hello, {name}"

[arskom-jenkins]

Can one of the admins verify this patch?

[plq]

Hey, this looks neat, i'd definitely merge it if it matures. Thanks for the effort!

Two points:

1. I can't allow a separate decorator. @rpc should assert that no *params are passed when it detects type-annotations in the function signature and continue reading type information from type annotations instead of *params.
2. I don't think we support calling @rpc without parens atm. So either the first example should be @rpc() and not @typed_rpc OR we should add another mode of operation that somehow checks that *params == (<member callable?>,) and proceeds as if the decorator is called before decorating (ie @rpc())

What do you think?

[ghandic]

So potentially just the first example but blended into the rpc decorator?

Funnily enough, I did that first, but then realized there were many other kwargs I hadn't seen before hence allowing it to have optional pass through kwargs

I think I understand what you mean, but if you could add some dummy examples it would help :)

[plq]

The following is OK:

class HelloWorldService(ServiceBase):
    @rpc(_is_async=True)
    def say_hello(
        ctx,
        name: Unicode,
        times: UnsignedInteger32,
        a: Iterable(Decimal),
        b: Iterable(Iterable(Decimal)),
    ) -> Iterable(Unicode):
        for _ in range(times):
            yield f"Hello, {name}"

The following should fail with: "*params must be empty when type annotations are used"

class HelloWorldService(ServiceBase):
    @rpc(Unicode, _is_async=True)
    def say_hello(
        ctx,
        name: Unicode,
        times: UnsignedInteger32,
        a: Iterable(Decimal),
        b: Iterable(Iterable(Decimal)),
    ) -> Iterable(Unicode):
        for _ in range(times):
            yield f"Hello, {name}"

The following usage of @rpc decorator (no parens) is not supported at the moment but we must make it work. That's 2nd point in my prev post

class HelloWorldService(ServiceBase):
    @rpc
    def say_hello(
        ctx,
        name: Unicode,
        times: UnsignedInteger32,
        a: Iterable(Decimal),
        b: Iterable(Iterable(Decimal)),
    ) -> Iterable(Unicode):
        for _ in range(times):
            yield f"Hello, {name}"

The following (with parens) should work same as above:

class HelloWorldService(ServiceBase):
    @rpc()
    def say_hello(
        ctx,
        name: Unicode,
        times: UnsignedInteger32,
        a: Iterable(Decimal),
        b: Iterable(Iterable(Decimal)),
    ) -> Iterable(Unicode):
        for _ in range(times):
            yield f"Hello, {name}"

How we handle missing data:

• Missing annotations for function arguments : "Missing type annotation for the nth argument"
• Missing annotations for function return type: The function does not return anything ie in C-speak it returns void ie just like having _returns omitted ie no error

The following falls under the first point above.

class HelloWorldService(ServiceBase):
    @rpc()
    def say_hello(
        ctx,
        name,
        times: UnsignedInteger32,
        a: Iterable(Decimal),
        b: Iterable(Iterable(Decimal)),
    ) -> Iterable(Unicode):
        for _ in range(times):
            yield f"Hello, {name}"

You needn't worry about **kparams, just pass them along.

[plq]

The following should fail with: "_returns must be omitted when type annotations are used. Please annotate the return type"

class HelloWorldService(ServiceBase):
    @rpc(_returns=Iterable(Unicode), _is_async=True)
    def say_hello(
        ctx,
        name: Unicode,
        times: UnsignedInteger32,
        a: Iterable(Decimal),
        b: Iterable(Iterable(Decimal)),
    ):
        for _ in range(times):
            yield f"Hello, {name}"

[ghandic]

I've added this exception handling, will try to add some tests

[ghandic]

I've blindly added some tests :D couldnt seem to get the run_tests.sh running on my machine - Is the jenkins pipeline visible? - I can only see the last run on Jenkins was 11 months ago? Was there a reason for not using github runners?

[plq]

run_tests.sh is quite involved -- it starts by downloading and compiling the desired cpython version, so requires all C tooling to be ready to go. The good news is you don't need it, just switch to your local virtualenv run the tests directly.

There is a readme for tests under spyne/test. It was embarrassingly out of date but I just updated it. Let me know if you still have questions.

Was there a reason for not using github runners?

Spyne project predates those niceties by at least a decade :)

[ghandic]

Hmmm python setup.py test

/Library/Frameworks/Python.framework/Versions/3.9/lib/python3.9/site-packages/setuptools/dist.py:487: UserWarning: Normalizing '2.15.0-alpha' to '2.15.0a0'
  warnings.warn(tmpl.format(**locals()))
running test
WARNING: Testing via this command is deprecated and will be removed in a future version. Users looking for a generic test entry point independent of test runner are encouraged to use tox.
running egg_info
writing spyne.egg-info/PKG-INFO
writing dependency_links to spyne.egg-info/dependency_links.txt
writing entry points to spyne.egg-info/entry_points.txt
writing requirements to spyne.egg-info/requires.txt
writing top-level names to spyne.egg-info/top_level.txt
reading manifest file 'spyne.egg-info/SOURCES.txt'
adding license file 'LICENSE'
writing manifest file 'spyne.egg-info/SOURCES.txt'
running build_ext
Test stage 1: Unit tests
Traceback (most recent call last):
  File "/Users/andrewchallis/Documents/jsf/spyne2/spyne/setup.py", line 261, in <module>
    setup(
  File "/Library/Frameworks/Python.framework/Versions/3.9/lib/python3.9/site-packages/setuptools/__init__.py", line 153, in setup
    return distutils.core.setup(**attrs)
  File "/Library/Frameworks/Python.framework/Versions/3.9/lib/python3.9/distutils/core.py", line 148, in setup
    dist.run_commands()
  File "/Library/Frameworks/Python.framework/Versions/3.9/lib/python3.9/distutils/dist.py", line 966, in run_commands
    self.run_command(cmd)
  File "/Library/Frameworks/Python.framework/Versions/3.9/lib/python3.9/distutils/dist.py", line 985, in run_command
    cmd_obj.run()
  File "/Library/Frameworks/Python.framework/Versions/3.9/lib/python3.9/site-packages/setuptools/command/test.py", line 223, in run
    self.run_tests()
  File "/Users/andrewchallis/Documents/jsf/spyne2/spyne/setup.py", line 226, in run_tests
    ret = call_pytest_subprocess(*tests, capture=self.capture) or ret
  File "/Users/andrewchallis/Documents/jsf/spyne2/spyne/setup.py", line 161, in call_pytest_subprocess
    return call_test(pytest.main, args, tests, env)
  File "/Users/andrewchallis/Documents/jsf/spyne2/spyne/setup.py", line 72, in call_test
    p.start()
  File "/Library/Frameworks/Python.framework/Versions/3.9/lib/python3.9/multiprocessing/process.py", line 121, in start
    self._popen = self._Popen(self)
  File "/Library/Frameworks/Python.framework/Versions/3.9/lib/python3.9/multiprocessing/context.py", line 224, in _Popen
    return _default_context.get_context().Process._Popen(process_obj)
  File "/Library/Frameworks/Python.framework/Versions/3.9/lib/python3.9/multiprocessing/context.py", line 284, in _Popen
    return Popen(process_obj)
  File "/Library/Frameworks/Python.framework/Versions/3.9/lib/python3.9/multiprocessing/popen_spawn_posix.py", line 32, in __init__
    super().__init__(process_obj)
  File "/Library/Frameworks/Python.framework/Versions/3.9/lib/python3.9/multiprocessing/popen_fork.py", line 19, in __init__
    self._launch(process_obj)
  File "/Library/Frameworks/Python.framework/Versions/3.9/lib/python3.9/multiprocessing/popen_spawn_posix.py", line 47, in _launch
    reduction.dump(process_obj, fp)
  File "/Library/Frameworks/Python.framework/Versions/3.9/lib/python3.9/multiprocessing/reduction.py", line 60, in dump
    ForkingPickler(file, protocol).dump(obj)
AttributeError: Can't pickle local object '_wrapper.<locals>._'
Traceback (most recent call last):                                                                    
  File "/Library/Frameworks/Python.framework/Versions/3.9/lib/python3.9/multiprocessing/resource_tracker.py", line 201, in main
/Library/Frameworks/Python.framework/Versions/3.9/lib/python3.9/multiprocessing/resource_tracker.py:216: UserWarning: resource_tracker: There appear to be 3 leaked semaphore objects to clean up at shutdown
  warnings.warn('resource_tracker: There appear to be %d '
/Library/Frameworks/Python.framework/Versions/3.9/lib/python3.9/multiprocessing/resource_tracker.py:229: UserWarning: resource_tracker: '/mp-tspay7jf': [Errno 2] No such file or directory
  warnings.warn('resource_tracker: %r: %s' % (name, e))
/Library/Frameworks/Python.framework/Versions/3.9/lib/python3.9/multiprocessing/resource_tracker.py:229: UserWarning: resource_tracker: '/mp-ejx7eias': [Errno 2] No such file or directory
  warnings.warn('resource_tracker: %r: %s' % (name, e))
/Library/Frameworks/Python.framework/Versions/3.9/lib/python3.9/multiprocessing/resource_tracker.py:229: UserWarning: resource_tracker: '/mp-37d3chxe': [Errno 2] No such file or directory
  warnings.warn('resource_tracker: %r: %s' % (name, e))
    cache[rtype].remove(name)
KeyError: '/mp-tspay7jf'
Traceback (most recent call last):
  File "/Library/Frameworks/Python.framework/Versions/3.9/lib/python3.9/multiprocessing/resource_tracker.py", line 201, in main
    cache[rtype].remove(name)
KeyError: '/mp-ejx7eias'
Traceback (most recent call last):
  File "/Library/Frameworks/Python.framework/Versions/3.9/lib/python3.9/multiprocessing/resource_tracker.py", line 201, in main
    cache[rtype].remove(name)
KeyError: '/mp-37d3chxe'

[ghandic]

Jenkins seems to fail to get the repo

ERROR: Error fetching remote repo 'origin'

[plq]

don't worry about these, just use pytest to run your specific tests

[ghandic]

Done ✅ - I needed to delete spyne/test/interop/test_django.py to run the tests as I get this error

ModuleNotFoundError: No module named 'rpctest'

[plq]

Done ✅

Looks good so far. Next, you need to fuse the functionality of @typed_rpc into @rpc and remove @typed_rpc from the public api. For example, no tests should use @typed_rpc , you should always use @rpc, even when testing annotated services.

I needed to delete spyne/test/interop/test_django.py to run the tests as I get this error

As said in spyne/test/README.md, You can avoid all that by calling pytest -v spyne/test/test_service.py -- or whatever test module you want to run

[ghandic]

So...

This following use the existing rpc functionality and should all be backward compatible

# All existing with no type annos

class HelloWorldService(ServiceBase):
    @rpc(Unicode,UnsignedInteger32, Iterable(Decimal), Iterable(Iterable(Decimal)), _returns=Iterable(Unicode), _is_async=True)
    def say_hello(ctx, name, times, a, b): 
        for _ in range(times):
            yield f"Hello, {name}"

# Support existing peoples code where they have full type annotations

class HelloWorldService(ServiceBase):
    @rpc(Unicode, UnsignedInteger32, Iterable(Decimal), Iterable(Iterable(Decimal)), _returns=Iterable(Unicode), _is_async=True)
    def say_hello(
        ctx,
        name: Unicode,
        times: UnsignedInteger32,
        a: Iterable(Decimal),
        b: Iterable(Iterable(Decimal)),
    ): # Note - no return defined
        for _ in range(times):
            yield f"Hello, {name}"

# Support existing peoples code where they have partial type annotations

class HelloWorldService(ServiceBase):
    @rpc(Unicode, UnsignedInteger32, Iterable(Decimal), Iterable(Iterable(Decimal)), _returns=Iterable(Unicode), _is_async=True)
    def say_hello(
        ctx,
        name: Unicode,
        times: UnsignedInteger32,
        a,
        b: Any,
    ): # Note - no return defined
        for _ in range(times):
            yield f"Hello, {name}"

BUT, if they dont supply any args to @rpc or, if they mismatch in length from the defined function.... Then we should error?

[plq]

OK I seem to have brought back all functionality lost to bitrot.

[plq]

All existing with no type annos

This should work as before

Support existing peoples code where they have full type annotations

The logic to support this will be too complicated, so no need to bother. Users need to choose between having type annotations in function definition or in the @rpc decorator. If type annotations are used, @rpc should raise an exception when it receives positional arguments (*params) OR _returns inside its keyword arguments (**kparams).

Support existing peoples code where they have partial type annotations

Same as above, this isn't supported.

[ghandic]

Ok, this next release won't be backward compatible for anyone who had type annotations in their functions?

[plq]

Ok, this next release won't be backward compatible for anyone who had type annotations in their functions?

Is this a thing? People have type annotations in their functions decorated by @rpc? Why?

[ghandic]

I'm going to say there's a chance people have done - allows for IDE autocomplete and able to be type checked using mypy

Perhaps some kind of depreciation notice, either way we will probably need to tackle the slightly more complex scenarios above

[plq]

OK, this is a good point. hmmm.

First, mypy's type annotators are not compatible with spyne's type annotators. So users will have to choose one or the other.

Maybe instead of

If type annotations are used, @rpc should raise an exception when it receives positional arguments (*params) OR _returns inside its keyword arguments (**kparams).

we should implement

If type annotations are used AND the used type annotators are Spyne's type annotators, @rpc should raise an exception when it receives positional arguments (*params) OR _returns inside its keyword arguments (**kparams).

Or, we could make sure Spyne's annotators are compatible with mypy's, though I imagine this would open a huge can of worms so I'm not entirely willing to go down that path.

A third option would be to merge this patch as-is (we'd have to rename @typed_rpc to @arpc which would be an alias for @rpc(_annotated=True)) and leave it to the user.

[plq]

Or, we could make sure Spyne's annotators are compatible with mypy's, though I imagine this would open a huge can of worms so I'm not entirely willing to go down that path.

Converting mypy types to spyne types while reading annotations inside @rpc should be doable. But making sure spyne types imitate mypy types enough to be picked up by IDEs and the like would turn out to be a tad hairy, I imagine.

[ghandic]

Yes this was the next step after supporting type annotations inline - I played around with this mapping but having read your docs there are suggestions on things like float/double/decimal and I feel like most people will just say float without reading the docs where you suggest using decimal.

I think it might be best going forward to make the breaking change to be honest. So long as it's clear in the versioning and docs. It's an easy change for the user to remove redundant code (if they were type hinting in multiple locations) - they'll be happy for it.

[ghandic]

Any further thoughts?

[plq]

Sorry, yes, I think we should parse annotations like we originally wanted. If backwards compatibility becomes a problem, we can turn off reading annotations with eg by a new spyne.const.READ_ANNOTATIONS flag

[ghandic]

I've given it a go

• Moved the old implementation of @rpc to @_rpc
• Added in a const, this can be monkey patched by the user if they really want by the below
• Added depreciation warnings
• Backwards compatible if no annotations are given, will receive depreciation warning asking them to move the types

import spyne.const
spyne.const.READ_ANNOTATIONS = False

# Now import rest of spyne functionality
from spyne import (
    Decimal,
    Iterable,
    ServiceBase,
    UnsignedInteger32,
)
from spyne import rpc

class HelloWorldService(ServiceBase):
    @rpc
    def say_hello(
        ctx,
        name: UnsignedInteger32,
        times: UnsignedInteger32,
        a: Iterable(Decimal),
        b: Iterable(Iterable(Decimal)),
    ) -> None:
        for _ in range(times):
            yield f"Hello, {name}"

[plq]

Besides the PEP8 violations, this looks good to me. If you could make sure lines don't exceed 80 characters, I can merge this

[ghandic]

Ran through black, but looks like it changed quite a bit to align with PEP8

[ghandic]

Might be good to add some doccos too, not sure where the best place to add them is, or what your plan is going forward with doccos - latest doccos still say they're WIP?

[plq]

Ran through black, but looks like it changed quite a bit to align with PEP8

Huh, this is a mess. Can't you simply break lines manually?

[plq]

Might be good to add some doccos too, not sure where the best place to add them is, or what your plan is going forward with doccos - latest doccos still say they're WIP?

The apidocs are all we have. You are free to add additional material under the docs section though.

[ghandic]

Huh, this is a mess. Can't you simply break lines manually?

I can, though I'd reccomend using something like black over your whole repo to align and enforce a standard.

Maybe a seperate PR ?

[ghandic]

done ✅

[ghandic]

Anything remaining for this?

[plq]

Sorry, release crunch at $DAYJOB, going to look at this first thing I find some time