qubole/qds-sdk-py

semantics of cmd.is_done() and cmd.is_success() seems incorrect

vogxn opened this issue · 1 comments

When querying for the status of a cmd I was using cmd.is_done("done") and cmd.is_success("done") to see if the cmd is completed and in done state AND cmd is completed and is successful. However these methods simply tell whether the status passed is a 'done' status or a 'success' status. This is an incorrect implementation. Ideally no status need be passed and cmd.is_Xxx() methods are expected to return whether the cmd reached that particular status in its transition or not.

This is the related code block in qds-sdk-py

commands.py
    @staticmethod
    def is_done(status):
        """
        Does the status represent a completed command
        Args:
            ``status``: a status string

        Returns:
            True/False
        """
        return (status == "cancelled" or status == "done" or status == "error")

    @staticmethod
    def is_success(status):
        return (status == "done")

they are used as helper functions when the status is already available.

we can add new instance methods in addition. i am not sure about whether
they can be named the same.

On Tue, Nov 18, 2014 at 2:09 PM, Prasanna Santhanam <
notifications@github.com> wrote:

When querying for the status of a cmd I was using cmd.is_done("done") and
cmd.is_success("done") to see if the cmd is completed and in done state
AND cmd is completed and is successful. However these methods simply tell
whether the status passed is a 'done' status or a 'success' status. This is
an incorrect implementation. Ideally no status need be passed and
cmd.is_Xxx() methods are expected to return whether the cmd reached that
particular status in its transition or not.

This is the related code block in qds-sdk-py

commands.py
@staticmethod
def is_done(status):
""" Does the status represent a completed command Args: status: a status string Returns: True/False """
return (status == "cancelled" or status == "done" or status == "error")

@staticmethod
def is_success(status):
    return (status == "done")


Reply to this email directly or view it on GitHub
#69.