web2py / py4web

Other
263 stars 129 forks source link

Suggestion: Define interfaces (e. g. for fixtures) #676

Closed fvicent closed 2 years ago

fvicent commented 2 years ago

Hi there! Since the project is in an early stage, I would suggest to include some interfaces wherever the framework expects an object with specific attributes and methods, which is the case for fixtures. Using the battle-tested zope.interface, which has been around for a while, an IFixture interface could be defined as:

from zope import interface

class IFixture(interface.Interface):

    def on_request():
        """
        Called when a request arrives.
        """

    def on_error():
        """
        Called when a request errors.
        """

    def on_success(status: int):
        """
        Called when a request is successful.
        """

    def finalize():
        """
        Called in any case at the end of a request.
        """

    def transform(output, shared_data):
        """
        Transforms the output, for example to apply template
        """

(The proper type annotations could be also included.) With such an approach, py4web users can check whether their custom fixtures comply with the contract (in their tests), and the framework itself can use the interface as a type annotation wherever a function expects a fixture object. Interfaces are self-documented and can even be recognized by mypy via the mypy-zope plugin (which is a work-in-progress though). The world is better with interfaces! What do you think?

mdipierro commented 2 years ago

Honestly i do not find this useful because a fixture must extend Fixture so already has those methods. No need to declare it must have them if they already exist.

On Sat, Jan 15, 2022, 17:41 Francisco Vicent @.***> wrote:

Hi there! Since the project is in an early stage, I would suggest to include some interfaces wherever the framework expects an object with specific attributes and methods, which is the case for fixtures. Using the battle-tested zope.interface https://github.com/zopefoundation/zope.interface, which has been around for a while, an IFixture interface could be defined as:

from zope import interface

class IFixture(interface.Interface):

def on_request():
    """
    Called when a request arrives.
    """

def on_error():
    """
    Called when a request errors.
    """

def on_success(status: int):
    """
    Called when a request is successful.
    """

def finalize():
    """
    Called in any case at the end of a request.
    """

def transform(output, shared_data):
    """
    Transforms the output, for example to apply template
    """
    return output

(The proper type annotations could be also included.) With such an approach, py4web users can check whether their custom fixtures comply with the contract (in their tests), and the framework itself can use the interface as a type annotation wherever a function expects a fixture object. Interfaces are self-documented and can even be recognized by mypy via the mypy-zope plugin (which is a work-in-progress though). The world is better with interfaces! What do you think?

— Reply to this email directly, view it on GitHub https://github.com/web2py/py4web/issues/676, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAHLZT4EPTAYTSMYYFGMB6TUWIO5NANCNFSM5MBVMZZQ . Triage notifications on the go with GitHub Mobile for iOS https://apps.apple.com/app/apple-store/id1477376905?ct=notification-email&mt=8&pt=524675 or Android https://play.google.com/store/apps/details?id=com.github.android&referrer=utm_campaign%3Dnotification-email%26utm_medium%3Demail%26utm_source%3Dgithub.

You are receiving this because you are subscribed to this thread.Message ID: @.***>

fvicent commented 2 years ago

They already exist but you're supposed to replace them, which is where you might break things. You could immediately catch those errors in your tests with interfaces. The way I see it, the current Fixture class is a mix between a base class (an implementation) and a (sort of) interface that would be better to decouple.

mdipierro commented 2 years ago

I am going to close this because I think it is not necessary