Let qiita_client handle file fetching and pushing to/from Qiita central, instead of plugins

[sjanssen2]

Hi @antgonza

Overview

This is a follow up PR to #57. When I worked on integrating the new functionality into multiple existing plugins, I came across several issues. After testing some alternative ideas, I came to the conclusion that it might be best, if I outsource the actual file transfer (only if using protocol "https") into qiita_client instead of touching each plugin's code.

Therefore, I identified three central locations, one for fetching files from central to plugin and two to push newly computed results to central:

1. push:
   1. ArtifactInfo: https://github.com/jlab/qiita_client/blob/2321dbe973df0f17f4151c69ce73fa83afa017b6/qiita_client/plugin.py#L133-L144
      A plugin consists of one or multiple QiitaCommands which get eventually called remotely from Qiita. Instead of "just" returning it's result in the __call__ function, I now traverse the results and push all registered files from the plugin to Qiita central. By default, i.e. using existing "filesystem" protocol, nothing will be moved, as push_file_to_central simply returns the provided filepath: https://github.com/jlab/qiita_client/blob/16671cabee644a1ddc2ebca7785e9a0fed785525/qiita_client/qiita_client.py#L839-L840
   2. patch html_summary: for QiitaTypePlugins, a patch call can update filepaths that point to html and support files in the database. To actually make these files available, they need to get pushed from plugin to central additionally. I am doing this, by testing which API endpoint is called via the general method patch and push files to Qiita central if path == ' /html_summary': https://github.com/jlab/qiita_client/blob/2321dbe973df0f17f4151c69ce73fa83afa017b6/qiita_client/qiita_client.py#L584-L602
2. fetch: Similarly, a plugin is requesting information about a) an artifact and b) metadata (prep- or sample) through get calls on two different API endpoints here: https://github.com/jlab/qiita_client/blob/2321dbe973df0f17f4151c69ce73fa83afa017b6/qiita_client/qiita_client.py#L485-L504, where I now inject testing for plugincoupling protocol and the two API endpoints and eventually fetch requested artifact files from central prior to triggering the plugin QiitaCommands.

I furthermore realized that e.g. qtp-job-output-folder operates not on individual filepaths, managed by the postgred DB, but on while black box directories. Therefore, I needed to extend the fetch/push mechanism accordingly, which requires to zip/unzip folder-content to be transported as one "file" through GET and POST requests. In addition, qtp-job-output-folder also needs to clean up test files that have been pushed to Qiita central; thus I had to add a delete_file_from_central function: https://github.com/jlab/qiita_client/blob/2321dbe973df0f17f4151c69ce73fa83afa017b6/qiita_client/qiita_client.py#L988 but still am worried about abuse and thus only make it available if Qiita central is in testmode. These additions also require changes in the qiita_pet/handlers/cloud_handlers endpoints of qiita central. The according PR is qiita-spots/qiita#3483

Minor solved issues

1. Plugin tests do not read the plugin configuration file, as the "commands" are directly called, instead of creating a new plugin job. Thus, a different "plugin_coupling" protocol - as stored in the config file - is not properly used for the test. To fix this, I changed the PluginTestCase class of the qiita_client to also check the environment variable QIITA_PLUGINCOUPLING. Thus, by setting this env, tests will enable different protocols even though they don't adhere to the value in the generated plugin configuration file.
2. As we use qiita_client in qp-target-gene, which operates on old python2, we need to make the makedirs function compatible. The argument exist_ok did not exist in py2.

Result

I am here testing the integration of qiita, nginx, and 8 plugins coupled through the new https protocol:
To see if changes negatively affect the traditional filesystem coupling, i.e. operate as is, I test this as well:

Roadmap

Once PRs for qiita and qiita_client are merged, I will create according PRs for the 8 plugins I am using so far. They have currently modified github actions to use my development branches; thus they need to be adopted first. An overview is here: jlab/qiita-keycloak#49 You might notice in the PRs of the plugins, that no change of the productive plugin code is necessary. However, tests need to be modified, as they often locally create some tests files, assuming direct access to BASE_DATA_DIR, which is not the case in my uncoupled tests. Annoyingly, the configuration_*** script has to be extended by the new plugincoupling argument.

Note: to pass tests, I changed the branch of Qiita, used in the github action. Revert to dev prior merge! https://github.com/jlab/qiita_client/blob/2321dbe973df0f17f4151c69ce73fa83afa017b6/.github/workflows/qiita-ci.yml#L55-L59

[antgonza]

Wow, really cool; thank you for your efforts on this. I'll review the other PRs and come back to this one after they are merged.

My only comments at this point are:

1. There are a few (at least a couple) where methods and variables are prepended by _ but they are used outside the methods (AKA not private); I would suggest renaming them.
2. There are a few other plugin-types (like BIOM support_files) that supports having folders.

[wasade]

What’s using 2.7?

[sjanssen2]

qp-target-gene: https://github.com/qiita-spots/qp-target-gene/blob/858436ee6d4a1fccd35faee0b156a12953b5774a/.github/workflows/qiita-plugin-ci.yml#L80

[wasade]

Ugh.