# getSchemaCatalogs() breaks Task encapsulation: remove it

XMLWordPrintable

#### Details

• Type: Improvement
• Status: Done
• Resolution: Done
• Fix Version/s: None
• Component/s:
• Labels:
• Story Points:
2
• Team:
Architecture

#### Description

getSchemaCatalogs is not used in gen3 so remove it.

The getSchemaCatalogs() method was added to Task to allow CmdLineTasks to introspect their subtasks for schemas they produce, but it requires the subtasks to report the schemas by butler dataset. This limits subtask reusability by locking them into producing a particular Butler dataset (or, as in DM-2191, requiring additional arguments from their parent task that they wouldn't need with a better design).

Instead, we should have per-subtask-slot interfaces (i.e. an interface for all subtasks that could fill a particular role in a CmdLineTask) for how the parent tasks should retrieve their schemas. This will require CmdLineTask subclasses to implement the writeSchemas method themselves, instead of inheriting an implementation from CmdLineTask itself.

#### Activity

Hide
Tim Jenness added a comment -

I've had a look through the code and it seems that we now define getSchemaCatalogs in a few places but there is no code that ever calls that method. There is also no usage of getAllSchemaCatalogs. Should all the getSchemaCatalogs methods be removed?

Show
Tim Jenness added a comment - I've had a look through the code and it seems that we now define getSchemaCatalogs in a few places but there is no code that ever calls that method. There is also no usage of getAllSchemaCatalogs. Should all the getSchemaCatalogs methods be removed?
Hide
Jim Bosch added a comment -

We can definitely remove getAllSchemaCatalogs; that's been fully superseded by initOutputs in Gen3.  And I think we can remove any getSchemaCatalogs methods that aren't currently being called (though we'd need to run ci_* to check, as I don't trust earlier test coverage at all on this point.

Show
Jim Bosch added a comment - We can definitely remove getAllSchemaCatalogs; that's been fully superseded by initOutputs in Gen3.  And I think we can remove any getSchemaCatalogs methods that aren't currently being called (though we'd need to run ci_* to check, as I don't trust earlier test coverage at all on this point.
Hide
Tim Jenness added a comment -

I'm running a quick Jenkins run to see what happens if getSchemaCatalogs disappears.

Show
Tim Jenness added a comment - I'm running a quick Jenkins run to see what happens if getSchemaCatalogs disappears.
Hide
Tim Jenness added a comment -

Jim Bosch ci_imsim and ci_hsc_gen3 built fine without getSchemaCatalogs so I think this is ready for review.

Show
Tim Jenness added a comment - Jim Bosch ci_imsim and ci_hsc_gen3 built fine without getSchemaCatalogs so I think this is ready for review.
Hide
Jim Bosch added a comment -

Mind making the PRs so approving them works properly?

Show
Jim Bosch added a comment - Mind making the PRs so approving them works properly?
Hide
Tim Jenness added a comment -

Sorry. Done now.

Show
Tim Jenness added a comment - Sorry. Done now.
Hide
Jim Bosch added a comment -

Looks good, and good luck with the unrelated MyPy failures in pipe_base.

Show
Jim Bosch added a comment - Looks good, and good luck with the unrelated MyPy failures in pipe_base.

#### People

Assignee:
Tim Jenness
Reporter:
Jim Bosch
Reviewers:
Jim Bosch
Watchers:
Jim Bosch, Tim Jenness