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

Status base class #58

Merged
merged 16 commits into from May 27, 2014
Merged

Status base class #58

merged 16 commits into from May 27, 2014

Conversation

squirrelo
Copy link
Contributor

This adds a decorator to the QiitaStatusObject that can check what the status of the object is and stop functions from running if not correct. examples:

class Study(QiitaStatusObject):
...
    @self.check_status("public", exclude=True)
    def delete_something(self):
        ...

Would only allow deletions if the study was not public. Makes for a nice way of having data integrity with everything still not living in memory.

This needed some changes to the database, mainly what the names of the status tables are called and changing every status column to be named status. Only affected the study_status, required_sample_info, and sample_status tables.

This also adds a subtle change to the main QiitaObject - a _table variable that will hold the name of the table that the object's id comes from, e.g. for the Study object we would have self._table = "study".


Parameters
----------
status: str
Copy link
Contributor

Choose a reason for hiding this comment

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

I think it will make sense to accept a list of str, so we can check that the status is in such list.

@josenavas
Copy link
Contributor

2 minor comments @squirrelo

Thanks!!!

@squirrelo
Copy link
Contributor Author

Thanks @josenavas I've integrated the changes.

@squirrelo squirrelo added this to the DB Interoperability milestone May 26, 2014
Example:
foo: foo_status_id ----> foo_status: foo_status_id, status
"""
if isinstance(status, str):
Copy link
Contributor

Choose a reason for hiding this comment

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

If you are going to do the conversion anyways, I would say that you just should accept a list, not a string and a list.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Eh, to me it's easier and more useful to accept both and just do the conversion. That way if you are doing just one status type you can forget the list and it still works.

dbstatus = conn.execute_fetchone(sql, (self._id, ))[0]

def wrapped_f(*args):
if (exclude and dbstatus not in status)or(not exclude and
Copy link
Contributor

Choose a reason for hiding this comment

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

spaces around or

Copy link
Contributor

Choose a reason for hiding this comment

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

it looks like exclude doesn't matter, why not just dbstatus not in status or dbstatus in status... but, under what situation would this not evaluate True?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Exclude is a flag on whether or not you want to have the database status in the list or not in the list (excluded).
The way this evaluates, when exclude is true dbstatus must not be in database, and if exclude is false, dbstatus must be in database.

@coveralls
Copy link

Coverage Status

Changes Unknown when pulling d8de6cc on squirrelo:status_base_class into * on biocore:master*.

@coveralls
Copy link

Coverage Status

Changes Unknown when pulling 80ecedf on squirrelo:status_base_class into * on biocore:master*.

@coveralls
Copy link

Coverage Status

Changes Unknown when pulling 8ee6d86 on squirrelo:status_base_class into * on biocore:master*.

@coveralls
Copy link

Coverage Status

Changes Unknown when pulling f76344a on squirrelo:status_base_class into * on biocore:master*.

@coveralls
Copy link

Coverage Status

Changes Unknown when pulling 4405814 on squirrelo:status_base_class into * on biocore:master*.

josenavas added a commit that referenced this pull request May 27, 2014
@josenavas josenavas merged commit c05e85d into qiita-spots:master May 27, 2014
@josenavas josenavas deleted the status_base_class branch May 27, 2014 23:06
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants