Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Proof of concept: reject invalid inherited methods #783

Closed

Commits on Jul 14, 2023

  1. Proof of concept: reject invalid inherited methods

    AuthClient is gaining a number of methods related to authenticated
    usages which require token auth. These methods will not be functional
    for NativeAppAuthClient and ConfidentialAppAuthClient.
    
    Because it is difficult to make a backwards-compatible change which
    alters the inheritance relationship for AuthClient and the other
    client types, it may be worth exploring other methods for ensuring
    that these "never functional" methods provide clear user-feedback that
    they do not work.
    Possibly -- arguably -- the inheritance relationship that produces
    `isinstance(NativeAppAuthClient("foo"), AuthClient)` could be
    considered outside of our public interfaces. That would leave us free
    to make all three client types inherit from a new base
    (`BaseAuthClient`?) and attach the new methods only to `AuthClient`.
    However, if we consider this part of the public interface, then an
    alternative solution is needed.
    
    This changeset introduces a class decorator
    (`globus_sdk.utils.replace_notimplemented_methods`) which replaces
    named inherited methods with stubs which raise `NotImplementedError`
    with a generated message. The result is a slightly awkward trace, but
    a clear error on bad usage:
    
        >>> nc.get_identities(ids="foo")
        Traceback (most recent call last):
          File "<stdin>", line 1, in <module>
          File ".../foo/bar/globus_sdk/utils.py", line 34, in injected_method
            raise NotImplementedError(
        NotImplementedError: NativeAppAuthClient does not support get_identities
    
    If the methodology is considered sound, we can decorate
    NativeAppAuthClient and ConfidentialAppAuthClient with
    known-unsupported method names for SDK v3.x and plan to change the
    inheritance structure in v4.0 . A full enumeration of supported and
    unsupported usages would be needed to make this feasible, along with a
    detailed changelog enumerating the now NotImplementedError-raising
    methods.
    sirosen committed Jul 14, 2023
    Configuration menu
    Copy the full SHA
    51eb376 View commit details
    Browse the repository at this point in the history