Skip to content

Commit

Permalink
For #28441: added raise_hell to _ensure_support()
Browse files Browse the repository at this point in the history
  • Loading branch information
@nemoDreamer
nemoDreamer committed Mar 25, 2015
1 parent e406d33 commit 5f590f6
Showing 1 changed file with 9 additions and 5 deletions.
14 changes: 9 additions & 5 deletions shotgun_api3/shotgun.py
View file
Original file line number Diff line number Diff line change
Expand Up @@ -137,7 +137,7 @@ def __init__(self, host, meta):
self._ensure_json_supported()


def _ensure_support(self, feature):
def _ensure_support(self, feature, raise_hell=True):
"""Checks the server version supports a given feature, raises an
exception if it does not.
Expand All @@ -147,10 +147,14 @@ def _ensure_support(self, feature):
"""

if not self.version or self.version < feature['version']:
raise ShotgunError(
"%s requires server version %s or higher, "\
"server is %s" % (feature['label'], _version_str(feature['version']), _version_str(self.version))
)
if raise_hell:
raise ShotgunError(
"%s requires server version %s or higher, "\
"server is %s" % (feature['label'], _version_str(feature['version']), _version_str(self.version))
)
return False
else:
return True


def _ensure_json_supported(self):
Expand Down

3 comments on commit 5f590f6

@manneohrstrom
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hey @nemoDreamer are there any cases where this raise_hell will be false? Is that something that we anticipate would happen in the future or maybe you have in another feature branch? (i checked on master, and it looks like it is not being used). If we keep it, an updated docstring would be fab! And maybe it could be renamed to something slightly more descriptive, like raise_hell_on_error or raise_exception or something? Sorry if you think I am being too picky! :) The _ensure_support is a totally awesome piece of cleanup in the API. We need more of those! :) Thanks!!

@manneohrstrom
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Also, there are lots of places where we have code like this:

    def unfollow(self, user, entity):

        if not self.server_caps.version or self.server_caps.version < (5, 1, 22):
            raise ShotgunError("Follow support requires server version 5.2 or "\
                "higher, server is %s" % (self.server_caps.version,))

Would be really sweet if we could use _ensure_support for all of these guys too!

@nemoDreamer
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yup, that was the plan ;)
I also added the "raise hell", so we could use the same functions to test silently.

Please sign in to comment.