Tripal BioDB Integration
Closed this issue · 15 comments
Valentin submitted a new Database API for Tripal which extends Drupal's native Database API with functionality specifically needed for Biological PostgreSQL databases including Tripal. The original effort for integration was in #131.
The original PR was closed and a new branch was created in this repository: tripal_biodb to facilitate closer collaboration on full integration of this submission into Tripal core.
All discussion on this integration will occur here.
The current next steps are to get tests working again on TripalDocker. I made an initial commit simply enabling the Tripal BioDB in the docker... however, in my local version, this is causing a syntax error.
@guignonv can you double-check that the most recent version of your API is in the t4d8::tripal_biodb branch? Note: if there is a difference feel free to make a PR to t4d8::tripal_biodb and immediately merge -no need for review for contributions from you on this branch :-)
I intend to look more into this on Tuesday.
I pushed some fixes on parseTableDdlToDrupal but I don't think it is related to the issue you're facing. I'll try to work on this on Friday.
It seems I'm using a more recent version of PHP (7.4) on my server than the one used in the docker (7.3). In BioDb module (and others...) I used a lot of new PHP syntactic sugar not available in PHP 7.3. I'll try fix that to be 7.3 compatible...
Commit 02b6b05 should pass the tests now. At least, they do on the docker run on my server.
Looks like a problem with Tripal Docker, not the tests. It can't seem to find the Tripal BioDB module which tells me it is not running on the local version of Tripal in this branch... 🤔 I'm looking into it
Ok, Tripal Docker is now running the local version of Tripal in the branch, we're able to install tripal_biodb in the docker build and the drush commands have been updated to use your new Installer + Remover Tasks! 🎉
However, we're still running into a weird issue shown here: https://github.com/tripal/t4d8/runs/4171830088?check_suite_focus=true#step:3:3218
The following module(s) will be enabled: devel, tripal, tripal_biodb, tripal_chado
// Do you want to continue?: yes.
[success] Successfully enabled: devel, tripal, tripal_biodb, tripal_chado
Installing chado version 1.3 in a schema named "chado"
[warning] array_key_exists(): The first argument should be either a string or an integer ChadoInstaller.php:69
[error] That requested version (1.3) is not supported by this installer.In ChadoInstaller.php line 72:
That requested version (1.3) is not supported by this installer.
I'm out of time for the week :-( Do you have any thoughts/ideas?
Note: You should be able to test locally by running the chado drush commands :-)
Looks like it was a variable-type issue. I had passed in a float and your installer expected a string. I added a couple more checks to ensure a string value :-)
Ok, the tests are passing on TripalDocker now 🎉
See this set of Github actions which show them all passing on the build tripaldocker for Drupal 9.0.x-dev, 9.1.x-dev and 9.2.x-dev
@risharde The chado prepare functionality which used to be at tripal_chado/src/Services/ChadoPrepare.php has been moved to tripal_chado/src/Task/ChadoPreparer.php to use the new task API in branch tripal-biodb. You're going to need to move your importer-based code into this new file so I suggest pulling the tripal-biodb branch into PR #167 -especially since you'll be using Tripal BioDB for the connection information.
@guignonv As part of updating the prepare functionality in your task to match the new behaviour, I've noticed you do not use the Tripal Logger. Is there a reason behind that?
Using the \Psr\Log\LoggerInterface logging channel you did causes all the output to be added to the Drupal > Admin > Reports > Recent Log Messages listing with no association to the task which not only fills up the log but is also not ideal for debugging a task gone wrong.
So I'm just wondering if this is some integration that just needs to be done (i.e. switching it to the existing Tripal Logger) or is there something the existing logger is missing that needs to be improved first? I do notice that we still have a Tripal Job being created so we can easily tie it into there for the logger to save the messages to a central space even if you decide to trigger your task outside the Tripal Job interface.
We have a PR merging in the first version of this functionality 🎉
PR #182
We decided to move this functionality into 9.x-4.x as it is stable, we are committed to it and other branches/functionality require this API to move forward.
Here are the current items left to be done after the above-mentioned PR is merged:
Additionally, the documentation for maintenance and usage will be expanded and move into the core spaces of the existing Tripal 4 documentation
I have a fix for the 9.4.x error in tv4-biodb-drupal9.4.x-error. Just waiting on tests :-)
@guignonv As part of updating the prepare functionality in your task to match the new behaviour, I've noticed you do not use the Tripal Logger. Is there a reason behind that?
Using the \Psr\Log\LoggerInterface logging channel you did causes all the output to be added to the Drupal > Admin > Reports > Recent Log Messages listing with no association to the task which not only fills up the log but is also not ideal for debugging a task gone wrong.
So I'm just wondering if this is some integration that just needs to be done (i.e. switching it to the existing Tripal Logger) or is there something the existing logger is missing that needs to be improved first? I do notice that we still have a Tripal Job being created so we can easily tie it into there for the logger to save the messages to a central space even if you decide to trigger your task outside the Tripal Job interface.
I was just using Drupal logger by default. If you think Triapl Logger is better, I've got nothing against that. :) The code can be adapted to use Tripal Logger; I see nothing that could go wrong.
I'm working on the issue you reported when using the query builder.
$connection = \Drupal::service('tripal_chado.database');
$connection->setSchemaName('chado');
$connection->addExtraSchema('chado2');
$query = $connection->select('2:feature', 'f');
$query->join('organism', 'o', 'o.organism_id = f.organism_id');
$query->join('cvterm', 'cvt', 'cvt.cvterm_id = f.type_id');
$query->condition('o.genus', 'Lens', '=');
$query->condition('cvt.name', 'chromosome', '=');
$query->fields('f', ['feature_id', 'name', 'uniquename']);
$query->fields('o', ['genus', 'species', 'common_name']);
$result = $query->execute();
foreach ($result as $record) {
dpm($record);
}
Giving the error:
SQLSTATE[42P01]: Undefined table: 7 ERROR: relation "2feature" does not exist LINE 3: "2feature" "f" ^: SELECT "f"."feature_id" AS "feature_id", "f"."name" AS "name", "f"."uniquename" AS "uniquename", "o"."genus" AS "genus", "o"."species" AS "species", "o"."common_name" AS "common_name" FROM "2feature" "f" INNER JOIN "organism" "o" ON o.organism_id = f.organism_id INNER JOIN "cvterm" "cvt" ON cvt.cvterm_id = f.type_id WHERE ("o"."genus" = :db_condition_placeholder_0) AND ("cvt"."name" = :db_condition_placeholder_1); Array ( [:db_condition_placeholder_0] => Lens [:db_condition_placeholder_1] => chromosome )