Skip to content

Delete preprocess data #1042

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

Merged
merged 6 commits into from
Apr 8, 2015
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
71 changes: 69 additions & 2 deletions qiita_db/data.py
Original file line number Diff line number Diff line change
Expand Up @@ -103,7 +103,7 @@ class BaseData(QiitaObject):
--------
RawData
PreprocessedData
PreprocessedData
ProcessedData
"""
_filepath_table = "filepath"

Expand Down Expand Up @@ -807,6 +807,73 @@ def create(cls, study, preprocessed_params_table, preprocessed_params_id,
ppd.add_filepaths(filepaths, conn_handler)
return ppd

@classmethod
def delete(cls, ppd_id):
"""Removes the preprocessed data with id ppd_id

Parameters
----------
ppd_id : int
The preprocessed data id

Raises
------
QiitaDBStatusError
If the preprocessed data status is not sandbox or if the
preprocessed data EBI and VAMPS submission is not in a valid state
['not submitted', 'failed']
QiitaDBError
If the preprocessed data has been processed
"""
valid_submission_states = ['not submitted', 'failed']
ppd = cls(ppd_id)
if ppd.status != 'sandbox':
raise QiitaDBStatusError(
"Illegal operation on non sandboxed preprocessed data")
elif ppd.submitted_to_vamps_status() not in valid_submission_states:
raise QiitaDBStatusError(
"Illegal operation. This preprocessed data has or is being "
"added to VAMPS.")
elif ppd.submitted_to_insdc_status() not in valid_submission_states:
raise QiitaDBStatusError(
"Illegal operation. This preprocessed data has or is being "
"added to EBI.")

conn_handler = SQLConnectionHandler()

processed_data = [str(n[0]) for n in conn_handler.execute_fetchall(
"SELECT processed_data_id FROM qiita.preprocessed_processed_data "
"WHERE preprocessed_data_id = {0} ORDER BY "
"processed_data_id".format(ppd_id))]

if processed_data:
raise QiitaDBError(
"Preprocessed data %d cannot be removed because it was used "
"to generate the following processed data: %s" % (
ppd_id, ', '.join(processed_data)))

# delete
queue = "delete_preprocessed_data_%d" % ppd_id
conn_handler.create_queue(queue)

sql = ("DELETE FROM qiita.prep_template_preprocessed_data WHERE "
"preprocessed_data_id = {0}".format(ppd_id))
conn_handler.add_to_queue(queue, sql)

sql = ("DELETE FROM qiita.preprocessed_filepath WHERE "
"preprocessed_data_id = {0}".format(ppd_id))
conn_handler.add_to_queue(queue, sql)

sql = ("DELETE FROM qiita.study_preprocessed_data WHERE "
"preprocessed_data_id = {0}".format(ppd_id))
conn_handler.add_to_queue(queue, sql)

sql = ("DELETE FROM qiita.preprocessed_data WHERE "
"preprocessed_data_id = {0}".format(ppd_id))
conn_handler.add_to_queue(queue, sql)

conn_handler.execute_queue(queue)

@property
def processed_data(self):
r"""The processed data list generated from this preprocessed data"""
Expand Down Expand Up @@ -1275,7 +1342,7 @@ def delete(cls, processed_data_id):
"""
if cls(processed_data_id).status != 'sandbox':
raise QiitaDBStatusError(
"Illegal operation on non sandbox processed data")
"Illegal operation on non sandboxed processed data")

conn_handler = SQLConnectionHandler()

Expand Down
42 changes: 42 additions & 0 deletions qiita_db/test/test_data.py
Original file line number Diff line number Diff line change
Expand Up @@ -478,6 +478,48 @@ def test_create_data_type_only(self):
# preprocessed_data_id, filepath_id
self.assertEqual(obs, [[3, obs_id - 1], [3, obs_id]])

def test_delete_basic(self):
"""Correctly deletes a preprocessed data"""
# testing regular delete
ppd = PreprocessedData.create(
self.study, self.params_table,
self.params_id, self.filepaths, prep_template=self.prep_template,
ebi_submission_accession=self.ebi_submission_accession,
ebi_study_accession=self.ebi_study_accession)
PreprocessedData.delete(ppd.id)
Copy link
Contributor

Choose a reason for hiding this comment

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

This should have some SQL tests to make sure the delete actually removed all the rows expected from the DB.

Copy link
Member Author

Choose a reason for hiding this comment

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

I can add but I see them as useless as the test just after this one is to check that the deletion fails cause this object doesn't exist anymore, do you agree?

Copy link
Contributor

Choose a reason for hiding this comment

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

This is to make sure that the lines are all deleted out of the 4 or 5 tables, since the failure is only due to one row in one table.

Copy link
Member Author

Choose a reason for hiding this comment

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

Well, the lines are in a queue so if one fails, all fail, right?

Copy link
Contributor

Choose a reason for hiding this comment

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

It's for future-proofing to make sure the lines are always deleted no matter the code changes. If we change the code, we still need to make sure the functionality remains.

Copy link
Contributor

Choose a reason for hiding this comment

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

I see your point @antgonza. However, this won't work in the case that we move the "delete" function from a classmethod to an instance method. This is something that I've been thinking and I was planning to propose to the group, as I think it makes way more sense to have the delete function as an instance method rather than a classmethod.

Copy link
Member Author

Choose a reason for hiding this comment

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

If you feel like that's what we should do, IMOO and following the "rules" we discussed last week, that should be a separate issue. Anyway, the concern about having it as instance method is:

pd.delete()
pd.whatever = 55

Copy link
Contributor

Choose a reason for hiding this comment

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

Yes, I agree that it should be a separate issue. BTW, that concern is not solved with the current solution:

PreprocessedData.delete(ppd.id)
ppd.whatever = 55

I moved the discussion here: #1045

Copy link
Contributor

Choose a reason for hiding this comment

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

BTW, I think that @ElDeveloper enhancements should be implemented, as it is safer.

Copy link
Contributor

Choose a reason for hiding this comment

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

Agree with @ElDeveloper enhancements as well.


# testing that the deleted preprocessed data can't be instantiated
with self.assertRaises(QiitaDBUnknownIDError):
PreprocessedData(ppd.id)
# and for completeness testing that it raises an error if ID
# doesn't exist
with self.assertRaises(QiitaDBUnknownIDError):
PreprocessedData.delete(ppd.id)

# testing that we can not remove cause the preprocessed data != sandbox
with self.assertRaises(QiitaDBStatusError):
PreprocessedData.delete(1)

def test_delete_advanced(self):
# testing that we can not remove cause preprocessed data has been
# submitted to EBI or VAMPS
ppd = PreprocessedData.create(
self.study, self.params_table,
self.params_id, self.filepaths, prep_template=self.prep_template,
ebi_submission_accession=self.ebi_submission_accession,
ebi_study_accession=self.ebi_study_accession)

# fails due to VAMPS submission
ppd.update_vamps_status('success')
with self.assertRaises(QiitaDBStatusError):
PreprocessedData.delete(ppd.id)
ppd.update_vamps_status('failed')

# fails due to EBI submission
ppd.update_insdc_status('success', 'AAAA', 'AAAA')
with self.assertRaises(QiitaDBStatusError):
PreprocessedData.delete(ppd.id)

def test_create_error_dynamic_table(self):
"""Raises an error if the preprocessed_params_table does not exist"""
with self.assertRaises(IncompetentQiitaDeveloperError):
Expand Down
28 changes: 28 additions & 0 deletions qiita_pet/handlers/study_handlers/description_handlers.py
Original file line number Diff line number Diff line change
Expand Up @@ -697,6 +697,33 @@ def delete_prep_template(self, study, user, callback):

callback((msg, msg_level, 'raw_data_tab', prep_id, None))

def delete_preprocessed_data(self, study, user, callback):
"""Delete the selected preprocessed data

Parameters
----------
study : Study
The current study object
user : User
The current user object
callback : function
The callback function to call with the results once the processing
is done
"""
ppd_id = int(self.get_argument('preprocessed_data_id'))

try:
PreprocessedData.delete(ppd_id)
msg = ("Preprocessed data %d has been deleted" % ppd_id)
msg_level = "success"
ppd_id = None
except Exception as e:
msg = ("Couldn't remove preprocessed data %d: %s" %
(ppd_id, str(e)))
msg_level = "danger"

callback((msg, msg_level, 'preprocessed_data_tab', ppd_id, None))

def delete_processed_data(self, study, user, callback):
"""Delete the selected processed data

Expand Down Expand Up @@ -756,6 +783,7 @@ def post(self, study_id):
update_investigation_type=self.update_investigation_type,
delete_raw_data=self.delete_raw_data,
delete_prep_template=self.delete_prep_template,
delete_preprocessed_data=self.delete_preprocessed_data,
delete_processed_data=self.delete_processed_data)

# Get the action that we need to perform
Expand Down
20 changes: 20 additions & 0 deletions qiita_pet/templates/study_description.html
Original file line number Diff line number Diff line change
Expand Up @@ -222,6 +222,26 @@
}
}

function delete_preprocessed_data(preprocessed_data_id) {
if (confirm('Are you sure you want to delete preprocessed data ID: ' + preprocessed_data_id + '?')) {
var form = $("<form>")
.attr("action", window.location.href)
.attr("method", "post")
.append($("<input>")
.attr("type", "hidden")
.attr("name", "preprocessed_data_id")
.attr("value", preprocessed_data_id))
.append($("<input>")
.attr("type", "hidden")
.attr("name", "action")
.attr("value", "delete_preprocessed_data"));
$("body").append(form);
form.submit();
} else {
return false;
}
}

function delete_processed_data(processed_data_id) {
if (confirm('Are you sure you want to delete processed data ID: ' + processed_data_id + '?')) {
var form = $("<form>")
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@
<li><a href="#preprocessed_data_info_{{ppd_id}}" role="tab" data-toggle="tab">ID: {{ppd_id}}&nbsp;
<div class="{{class_icon1}}" style="color: {{color}};"></div>
<div class="{{class_icon2}}" style="color: {{color}};"></div>
<button class="close" title="Remove this preprocessed data" type="button" onclick="delete_preprocessed_data({{ppd_id}})">&nbsp; ×</button>
</a></li>
{% end %}
</ul>
Expand Down