Adding archive #2449

antgonza · 2017-12-27T13:15:55Z

This adds the basic archiving code. @sjanssen2 and @josenavas, could you take a look so we can continue working on it? Thanks!

Qiita Release December 22nd

…late-download Fixing download issue

codecov-io · 2017-12-27T16:51:12Z

Codecov Report

Merging #2449 into dev will increase coverage by 0.01%.
The diff coverage is 97.45%.

@@            Coverage Diff             @@
##              dev    #2449      +/-   ##
==========================================
+ Coverage   94.15%   94.17%   +0.01%     
==========================================
  Files         164      166       +2     
  Lines       19445    19558     +113     
==========================================
+ Hits        18308    18418     +110     
- Misses       1137     1140       +3

Impacted Files	Coverage Δ
qiita_db/handlers/archive.py	`100% <100%> (ø)`	⬆️
qiita_db/handlers/tests/test_archive.py	`84.37% <100%> (+6.11%)`	⬆️
qiita_db/handlers/tests/test_processing_job.py	`98.71% <100%> (+0.11%)`	⬆️
qiita_db/__init__.py	`100% <100%> (ø)`	⬆️
qiita_db/test/test_archive.py	`96.15% <96.15%> (ø)`
qiita_db/archive.py	`96.66% <96.66%> (ø)`

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 67a33dc...376d78f. Read the comment docs.

sjanssen2 · 2017-12-27T17:42:25Z

Hey @antgonza thanks for your efforts. I am travelling with my family right now who will get quite angry if I start staring into my laptop. I might find some time in two days

update dep versions to fix master

wasade · 2018-01-10T15:21:15Z

.travis.yml

 env:
  global:
-    - PYTHON_VERSION=2.7
+    - PYTHON_VERSION=3.5


Long story short: there is a bug in conda that the Qiime2 guys reported and this is the best way to install everything. See that these changes are in master and are passing.

wasade · 2018-01-10T15:22:04Z

qiita_db/archive.py

+
+        Raises
+        ------
+            ValueError


wasade · 2018-01-10T15:22:35Z

qiita_db/archive.py

+                If the Artifact type is not BIOM
+                If the artifact doesn't have a biom filepath
+        """
+        with qdb.sql_connection.TRN:


seems like most of the logic contained can be done outside of the context?

possible but I think it's better to leave it like this as some of these steps are actually accessing the DB so this assures that everything it's in the same transaction.

oh. implicit operations remote resource operations? :/ okay.

wasade · 2018-01-10T15:22:59Z

qiita_db/archive.py

+                If the artifact doesn't have a biom filepath
+        """
+        with qdb.sql_connection.TRN:
+            acmd = job.command


same ... 😄

off topic but how does a developer know whether a given qiita method (or property) will issue a db call?

I think the safest assumption is that any property/method from a Qiita object will issue a DB call ... as everything is store in there.

wasade · 2018-01-10T15:23:37Z

qiita_db/archive.py

+
+        Notes
+        -----
+            If archive_merging_scheme is None it will return all


extra indent

wasade · 2018-01-10T15:23:46Z

qiita_db/archive.py

+            feature values
+        """
+        with qdb.sql_connection.TRN:
+            extras = []


ya' know, same 😉

wasade · 2018-01-10T15:26:36Z

qiita_db/test/test_archive.py

+
+        # now let's tests that all the inserts happen as expected
+        exp = {
+            'featureA4': '{valuesA: vA, int: 1}',


are these supposed to be json? if so, they're not as the strings "vA" and "vB" are not quoted, nor are the keys

well, they are strings but supposedly represent json, I can change for test if you feel strongly about it.

if this is stuff generated from the code, then it certainly is not generating json. if this is supposed to be json, should the data contained in the insertion logic validate that the data are json?

antgonza

Thanks @wasade !

antgonza · 2018-01-10T15:28:04Z

.travis.yml

 env:
  global:
-    - PYTHON_VERSION=2.7
+    - PYTHON_VERSION=3.5


Long story short: there is a bug in conda that the Qiime2 guys reported and this is the best way to install everything. See that these changes are in master and are passing.

antgonza · 2018-01-10T15:28:15Z

qiita_db/archive.py

+
+        Raises
+        ------
+            ValueError


antgonza · 2018-01-10T15:29:40Z

qiita_db/archive.py

+                If the Artifact type is not BIOM
+                If the artifact doesn't have a biom filepath
+        """
+        with qdb.sql_connection.TRN:


possible but I think it's better to leave it like this as some of these steps are actually accessing the DB so this assures that everything it's in the same transaction.

antgonza · 2018-01-10T15:29:55Z

qiita_db/archive.py

+                If the artifact doesn't have a biom filepath
+        """
+        with qdb.sql_connection.TRN:
+            acmd = job.command


same ... 😄

antgonza · 2018-01-10T15:30:19Z

qiita_db/archive.py

+
+        Notes
+        -----
+            If archive_merging_scheme is None it will return all


antgonza · 2018-01-10T15:30:33Z

qiita_db/archive.py

+            feature values
+        """
+        with qdb.sql_connection.TRN:
+            extras = []


ya' know, same 😉

antgonza · 2018-01-10T15:32:15Z

qiita_db/test/test_archive.py

+
+        # now let's tests that all the inserts happen as expected
+        exp = {
+            'featureA4': '{valuesA: vA, int: 1}',


well, they are strings but supposedly represent json, I can change for test if you feel strongly about it.

josenavas

Minor comments

josenavas · 2018-01-11T21:41:20Z

qiita_db/archive.py

+    """
+
+    @classmethod
+    def _inserting_main_steps(cls, TRN, ms, features):


Minor but there is no need to pass the TRN object by parameter. The system is using the singleton pattern, so using "with ...TRN:" should be good.

josenavas · 2018-01-11T21:41:45Z

qiita_db/archive.py

+
+    Methods
+    -------
+    insert_from_biom


insert_from_artifact?

josenavas · 2018-01-11T21:46:02Z

qiita_db/test/test_archive.py

+        self.assertEqual(
+            str(err.exception), 'To archive artifact must be BIOM but FASTQ')
+
+        # 7 - to test error due to not filepath biom


This is testing that an artifact of type BIOM doesn't have a BIOM? that seems like it should never happen.

right, but I think an extra test never hurts ... anyway, want me to remove it?

antgonza

Thanks! @josenavas

antgonza · 2018-01-11T23:41:00Z

qiita_db/archive.py

+
+    Methods
+    -------
+    insert_from_biom


antgonza · 2018-01-11T23:41:05Z

qiita_db/archive.py

+    """
+
+    @classmethod
+    def _inserting_main_steps(cls, TRN, ms, features):


antgonza · 2018-01-11T23:41:53Z

qiita_db/test/test_archive.py

+        self.assertEqual(
+            str(err.exception), 'To archive artifact must be BIOM but FASTQ')
+
+        # 7 - to test error due to not filepath biom


right, but I think an extra test never hurts ... anyway, want me to remove it?

josenavas and others added 7 commits December 22, 2017 07:08

Merge pull request qiita-spots#2445 from biocore/dev

7e5d60f

Qiita Release December 22nd

Fixing download issue

e0b6e31

Merge pull request qiita-spots#2446 from josenavas/fixing-sample-temp…

7b9009e

…late-download Fixing download issue

adding archive base

8beb629

python=3.5

5c33745

PYTHON_VERSION=3.5

42fa986

Miniconda3-3.7.3

aa900f1

antgonza and others added 10 commits January 4, 2018 12:32

fix master

7e060fc

data.qiime2 -> raw.githubusercontent

8571fd0

try Miniconda3-4.3.31

7ac2994

minimizing changes

667e751

qiime2-2017.11 -> qiime2-2017.12

ddc2595

PYTHON_VERSION=3.5

9cda47e

rm conda update --yes conda

0db78d7

Merge pull request qiita-spots#2453 from antgonza/fix-master

e8894c4

update dep versions to fix master

merging with master

ed07b7d

getting closer to full functionality

20a86a8

wasade requested changes Jan 10, 2018

View reviewed changes

addresssing some of @wasade comments

a5d216f

antgonza commented Jan 10, 2018

View reviewed changes

antgonza added 4 commits January 11, 2018 07:24

addressing @wasade json comment

1b99b87

q2 install via yml

6c2a4ea

adding env to conda

39a5a9a

rm params from conda

0888b44

josenavas reviewed Jan 11, 2018

View reviewed changes

antgonza commented Jan 11, 2018

View reviewed changes

addressing @josenavas comments

376d78f

josenavas merged commit c453abe into qiita-spots:dev Jan 12, 2018

antgonza deleted the adding-archive branch February 17, 2018 15:04

Uh oh!

Adding archive #2449

Adding archive #2449

Uh oh!

Conversation

antgonza commented Dec 27, 2017

Uh oh!

codecov-io commented Dec 27, 2017 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

sjanssen2 commented Dec 27, 2017

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

antgonza left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

josenavas left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

codecov-io commented Dec 27, 2017 •

edited

Loading