Bug in Subread process (data:alignment:bam:subread)

[genialis]

Bug in Subread process (data:alignment:bam:subread)

[jkokosar]

@JureZmrzlikar As suggested, icount-demultiplex process is removed here: https://github.com/genialis/genialis-bio/pull/4/commits/c1d1e4431f1f48b76d8194a40ff9105570b3a2f2

[JenkoB]

True.

[JenkoB]

PR to fix: #449

[lukaw3d]

@jkokosar I thought I'd look for unused inputs in all processes, and here are the potential issues I found:

• alignment-subread doesnt use "PE_options.consensus_subreads"
• alignment-tophat2 doesnt use "PE_options.library_type"
• icount-demultiplex doesnt use "mismatches" and "minimum_length"
• bbduk-single doesnt use "trimming_par.minlength"
• bbduk-paired doesnt use "trimming_par.minlength"
• workflow-chip-seq doesnt use "annotation"

[lukaw3d]

If someone wants to add a pull-request validation (e.g. @tjanez || @dblenkus ) to find these, here's pseudocode of my javascript searcher:

get processes (last version per slug)
for each process:
    get a list of input field names (for fields inside groups use full path (e.g. "protocols.growth_protocol"))

    ignore fields named "advanced"

    str = (get process code string || stringify workflow json) + process.data_name
    if str doesn't contain one of the field names:
        print bad process
        unless one of ['test-disabled', 'test-hidden', 'test-all-fields-with-description', 'test-output', 'test-express']

[jkokosar]

@mzganec Please not the bbduk issues above. Please fix this in your future PR. Barbara will tackle the rest

[mstajdohar]

Did we fix this issues in processes? Did we write the test @lukaw3d suggested?

[JenkoB]

I have fixed following bugs:
alignment-subread doesnt use "PE_options.consensus_subreads"
alignment-tophat2 doesnt use "PE_options.library_type"
workflow-chip-seq doesnt use "annotation"

Process icount-demultiplex is not in use anywhere, so we have discussed with @jkokosar to remove it.

[mstajdohar]

Please remove icount-demultiplex as planned. What about bbduk-single and bbduk-paired?

[JenkoB]

ok. I don't know about bbduk. @mzganec do you still plan to improve bbduk process and also fix this two bugs?

[mzganec]

Yes.

[JenkoB]

@mstajdohar do we have any way how to remove icount-demultiplex process or any other process? This probably needs to be done manually at the time of redeploy or is other way?

[JureZmrzlikar]

I can add this into data migrations. And @jkokosar can remove it in his PR genialis-bio#4.

[mstajdohar]

It needs to be done manually. Admins can probably delete the process in ReSDK. The problem is if this process was used to create data objects. If such data objects exist, the system will not allow you to delete a process, since this data objects link to the process for details. We should discuss a solution with @dblenkus. Maybe we could introduce the ADD permission on the processes which would mean you are allowed to run the process. Now the VIEW permissions allows you to run and see a process. If we supported ADD for running, we could use the VIEW permission just for displaying process' details in the data page.

[dblenkus]

One option is to include this into migrations and automatically delete all data objects created with that process. If you want to check which objects will be deleted you can do this with ReSKD:

for p in res.process.filter(slug='icount-demultiplex'):
    print(res.data.filter(process=p.id))

But you cannot delete the process over the API as it doesn't support that. If this will be needed more often, it can be easily supported.

[JureZmrzlikar]

Removal of icount-demultiplex has already been included in migrations.

[mstajdohar]

@mzganec Did you fix bbduk? @dblenkus Did you remove icount-demultiplex?

[mzganec]

This is my next task.