Adds missing bits #1960
Adds missing bits #1960
Conversation
|
Changes Unknown when pulling c86d37d on josenavas:plugin-installation-activate-cmd into * on biocore:plugin-installation*. |
| self.finish() | ||
|
|
||
|
|
||
| class ReloadPluginAPItestHandler(OauthBaseHandler): |
There was a problem hiding this comment.
This is missing a test, right?
There was a problem hiding this comment.
is this method idempotent? if not, should it be?
There was a problem hiding this comment.
same question regarding CommandActivateHandler
There was a problem hiding this comment.
This method is idempotent as long as the files in qiita_config.plugin_dir do not change.
CommandActivateHandler is idempotent.
| @@ -281,6 +284,17 @@ def create(cls, software, name, description, parameters): | |||
| for at in atypes] | |||
| qdb.sql_connection.TRN.add(sql_type, sql_params, many=True) | |||
|
|
|||
| # Add the outputs to the command | |||
| if outputs: | |||
There was a problem hiding this comment.
Just want to confirm that now there are no tests without output. I think is fine due to how it work but want to double check it.
| @@ -425,6 +439,31 @@ def outputs(self): | |||
| qdb.sql_connection.TRN.add(sql, [self.id]) | |||
| return qdb.sql_connection.TRN.execute_fetchindex() | |||
|
|
|||
| @property | |||
| def active(self): | |||
There was a problem hiding this comment.
This doesn't have an actual tests, right?
There was a problem hiding this comment.
I tested it through the function test_activate so it is implicitly tested.
|
Changes Unknown when pulling afc8707 on josenavas:plugin-installation-activate-cmd into * on biocore:plugin-installation*. |
|
👍 , @wasade, could you review? Thanks. |
| The list filepath types that the new artifact type supports, and | ||
| if they're required or not in an artifact instance of this type | ||
|
|
||
| Raises |
There was a problem hiding this comment.
is the only type of unacceptable artifact a duplicate one?
There was a problem hiding this comment.
This is creating a new artifact type (not a new artifact) and I couldn't think in any other case where a new type is unacceptable.
| self.finish() | ||
|
|
||
|
|
||
| class ReloadPluginAPItestHandler(OauthBaseHandler): |
There was a problem hiding this comment.
is this method idempotent? if not, should it be?
| self.finish() | ||
|
|
||
|
|
||
| class ReloadPluginAPItestHandler(OauthBaseHandler): |
There was a problem hiding this comment.
same question regarding CommandActivateHandler
|
|
||
| obs = qdb.artifact.Artifact.types() | ||
| exp = [['BIOM', 'BIOM table'], | ||
| ['Demultiplexed', 'Demultiplexed and QC sequeneces'], |
There was a problem hiding this comment.
Good catch, opened #1961 cause this is a larger issue.
| (r"/qiita_db/artifacts/(.*)/", ArtifactHandler), | ||
| (r"/qiita_db/prep_template/(.*)/data/", PrepTemplateDataHandler), | ||
| (r"/qiita_db/prep_template/(.*)/", PrepTemplateDBHandler), | ||
| (r"/qiita_db/references/(.*)/", ReferenceHandler), | ||
| (r"/qiita_db/plugins/(.*)/(.*)/commands/(.*)/activate/", |
There was a problem hiding this comment.
just curious, why not do named regex? eg ?P<foo>[^/]+/?P<bar>[^/]+/?
There was a problem hiding this comment.
I'm not sure what is the benefit?
|
@wasade thanks for review. Merging so we can continue reviews. |
Adds the missing code to be able to successfully install commands and types and adds some endpoints so the plugins can correctly run the tests.