postgres 9.5->13.4 #3144
postgres 9.5->13.4 #3144
Conversation
| @@ -184,7 +184,12 @@ def __init__(self, id_): | |||
|
|
|||
| with qdb.sql_connection.TRN: | |||
| self._check_subclass() | |||
| if not self._check_id(id_): | |||
| try: | |||
There was a problem hiding this comment.
The main error with the new version pgsql-13.4 is that it actually verifies that the uuids are uuids (vs other kind of strings) and it raises a specific error. However, this is not used in qiita and I don't see this being used in the near future + most tests in the code actually use random strings as identifiers so it is better to simply try to check the id here and if it fails (for whatever reason) report that the id doesn't exist.
There was a problem hiding this comment.
Good to know, see suggestion below for being specific on the exception
| @@ -837,7 +837,7 @@ def _common_extend_steps(self, md_template): | |||
|
|
|||
| # check that we are within the limit of number of samples | |||
| ms = self.max_samples() | |||
| nsamples = len(existing_samples) + len(new_samples) + len(self) | |||
| nsamples = len(existing_samples) + len(new_samples) | |||
There was a problem hiding this comment.
This is "interesting" not sure how but here we are actually adding twice the existing samples: (1) len(existing_samples) and (2) len(self).
| @@ -60,6 +60,8 @@ def get(self): | |||
| continue | |||
|
|
|||
| for job in cmd.processing_jobs: | |||
| if job.hidden: | |||
There was a problem hiding this comment.
Not showing hidden jobs for admins
| @@ -42,7 +42,7 @@ def test_samples_detail(self): | |||
| obs = _sample_details(qiita_db.study.Study(1), | |||
| ['1.SKD7.640191', 'doesnotexist']) | |||
| self.assertEqual(len(obs), len(exp)) | |||
| self.assertEqual(obs, exp) | |||
| self.assertCountEqual(obs, exp) | |||
There was a problem hiding this comment.
remember in python3 this actually tests the list/array values and not only the count.
qiita_ware/private_plugin.py
Outdated
| @@ -291,7 +291,7 @@ def delete_sample_or_column(job): | |||
|
|
|||
|
|
|||
| def _delete_analysis_artifacts(analysis): | |||
| aids = [a.id for a in analysis.artifacts] | |||
| aids = list(set([a.id for a in analysis.artifacts])) | |||
There was a problem hiding this comment.
making sure we only get one.
| # making sure the analysis, first thing to delete, still exists | ||
| self.assertTrue(Analysis.exists(1)) | ||
|
|
||
| # delete everything from the EBI submissions and the processing job so |
There was a problem hiding this comment.
This test is actually extremely complicated because the "populate.sql" reuses files/values so when the code rolls back, it tries to overwrite the same files and it goes 🍌. Additionally, we are testing the study delete in a new study below. Thus, removing this section.
qiita_db/base.py
Outdated
| if not self._check_id(id_): | ||
| try: | ||
| _id = self._check_id(id_) | ||
| except Exception: |
There was a problem hiding this comment.
| except Exception: | |
| except psycopg2.errors.InvalidTextRepresentation: |
| @@ -837,7 +837,7 @@ def _common_extend_steps(self, md_template): | |||
|
|
|||
| # check that we are within the limit of number of samples | |||
| ms = self.max_samples() | |||
| nsamples = len(existing_samples) + len(new_samples) + len(self) | |||
| nsamples = len(curr_samples) + len(new_samples) | |||
There was a problem hiding this comment.
nsamples should be the total of samples a new file will have: current+new; however, the line before was adding existing_samples [the samples already in the prep that are in the new list] + the new samples + len(self) [the total number of samples in the prep].
qiita_pet/test/rest/test_study.py
Outdated
| @@ -155,8 +155,8 @@ def test_get_valid(self): | |||
| exp = {'is_public': False, | |||
| 'has_sample_information': True, | |||
| 'sample_information_has_warnings': False, | |||
| 'preparations': [{'id': 1, 'has_artifact': True}, | |||
| {'id': 2, 'has_artifact': True}] | |||
| 'preparations': [{'id': 2, 'has_artifact': True}, | |||
There was a problem hiding this comment.
if the test is sensitive to order from the database, would it make sense to sort observed and expected based on a key?
|
@wasade; I think this is ready - thank you for your reviews. |
No description provided.