Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Create SCCS workflow #77

Merged
merged 3 commits into from
Jan 16, 2025
Merged
Show file tree
Hide file tree
Changes from 2 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion dsc/cli.py
Original file line number Diff line number Diff line change
Expand Up @@ -70,7 +70,7 @@ def main(
workflow = workflow_class(
collection_handle=collection_handle,
batch_id=batch_id,
email_recipients=tuple(email_recipients.split(",")),
email_recipients=email_recipients.split(","),
s3_bucket=s3_bucket,
output_queue=output_queue,
)
Expand Down
5 changes: 3 additions & 2 deletions dsc/workflows/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@

from dsc.workflows.base import Workflow
from dsc.workflows.base.simple_csv import SimpleCSV
from dsc.workflows.demo import DemoWorkflow
from dsc.workflows.demo import Demo
from dsc.workflows.sccs import SCCS

__all__ = ["DemoWorkflow", "SimpleCSV", "Workflow"]
__all__ = ["SCCS", "Demo", "SimpleCSV", "Workflow"]
2 changes: 1 addition & 1 deletion dsc/workflows/base/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,7 @@ def __init__(
self,
collection_handle: str,
batch_id: str,
email_recipients: tuple[str, ...],
email_recipients: list[str],
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If the linting is passing then I guess this is all good!

s3_bucket: str | None = None,
output_queue: str | None = None,
) -> None:
Expand Down
4 changes: 2 additions & 2 deletions dsc/workflows/demo.py
Original file line number Diff line number Diff line change
@@ -1,8 +1,8 @@
from dsc.workflows.base.simple_csv import SimpleCSV


class DemoWorkflow(SimpleCSV):
class Demo(SimpleCSV):

workflow_name: str = "demo"
submission_system: str = "DSpace@MIT"
metadata_mapping_path: str = "tests/fixtures/demo_metadata_mapping.json"
metadata_mapping_path: str = "dsc/workflows/metadata_mapping/demo.json"
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,14 @@
"item_identifier": {
"source_field_name": "item_identifier",
"language": null,
"delimiter": ""
"delimiter": "",
"required": true
},
"dc.title": {
"source_field_name": "dc.title",
"language": "en_US",
"delimiter": "",
"required": true
},
"dc.publisher": {
"source_field_name": "dc.publisher",
Expand Down Expand Up @@ -34,11 +41,6 @@
"language": "",
"delimiter": ""
},
"dc.title": {
"source_field_name": "dc.title",
"language": "en_US",
"delimiter": ""
},
"dc.relation.journal": {
"source_field_name": "dc.relation.journal",
"language": "",
Expand Down
79 changes: 79 additions & 0 deletions dsc/workflows/metadata_mapping/sccs.json
Original file line number Diff line number Diff line change
@@ -0,0 +1,79 @@
{
"item_identifier": {
"source_field_name": "item_identifier",
"language": null,
"delimiter": "",
jonavellecuerdo marked this conversation as resolved.
Show resolved Hide resolved
"required": true
},
"dc.title": {
"source_field_name": "dc.title",
"language": "en_US",
"delimiter": "",
"required": true
},
"dc.publisher": {
"source_field_name": "dc.publisher",
"language": "en_US",
"delimiter": ""
},
"dc.identifier.mitlicense": {
"source_field_name": "dc.identifier.mitlicense",
"language": "en_US",
"delimiter": ""
},
"dc.eprint.version": {
"source_field_name": "dc.eprint.version",
"language": "en_US",
"delimiter": ""
},
"dc.type": {
"source_field_name": "dc.type",
"language": "en_US",
"delimiter": ""
},
"dc.source": {
"source_field_name": "dc.source",
"language": "en_US",
"delimiter": ""
},
"dc.contributor.author": {
"source_field_name": "dc.contributor.author",
"language": "en_US",
"delimiter": "|"
},
"dc.relation.isversionof": {
"source_field_name": "dc.relation.isversionof",
"language": "",
"delimiter": ""
},
"dc.relation.journal": {
"source_field_name": "dc.relation.journal",
"language": "",
"delimiter": ""
},
"dc.identifier.issn": {
"source_field_name": "dc.identifier.issn",
"language": "",
"delimiter": ""
},
"dc.date.issued": {
"source_field_name": "dc.date.issued",
"language": "",
"delimiter": ""
},
"dc.rights": {
"source_field_name": "dc.rights",
"language": "en_US",
"delimiter": ""
},
"dc.rights.uri": {
"source_field_name": "dc.rights.uri",
"language": "",
"delimiter": ""
},
"dc.description.sponsorship": {
"source_field_name": "dc.description.sponsorship",
"language": "en_US",
"delimiter": ""
}
}
13 changes: 13 additions & 0 deletions dsc/workflows/sccs.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,13 @@
from dsc.workflows import SimpleCSV


class SCCS(SimpleCSV):
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

When looking at a class this minimal, a couple of thoughts come to mind:

  1. Nice work on class design and inheritance! Clearly much is shared between workflows.
  2. What functionality / opinionation, if anything, could this workflow-specific class still provide?

For # 2, the first thing that comes to mind is validation or hooks of some kind. I don't think it's necessary now, but wondering if there might be room in the future for some kind of registration of validation hooks, that could be used to validate items created? reconciliation logic? maybe others? In theory, these could be called -- if defined -- during the normal flow of the base Workflows.

Sketch:

class MyWorkflow(SimpleCSV):
    
    workflow_name = "Mine!"
    metadata_mapping_path = "path/to/mapping.json"
    
    # defined as class attributes, lists pointing to methods
    reconciliation_hooks = [] # I don't have any reconciliation hooks...
    item_creation_hooks = [
        "confirm_title_elements" # reference to method defined on class
    ]
    
    @classmethod
    def confirm_title_elements(cls):
        # I will check things yielded by Workflow.item_submissions_iter()...
        pass

With a little bit of wiring, this might provide a way for a Workflow class to have some additional validation (or even functional) logic applied to the base flows.

This pattern could also probably be achieved through decorators, which could be even cleaner.

Sketch:

class MyWorkflow(SimpleCSV):
    
    workflow_name = "Mine!"
    metadata_mapping_path = "path/to/mapping.json"
    
    @item_creation_hook
    def confirm_title_elements(cls):
        # I will check things yielded by Workflow.item_submissions_iter()...
        pass
   
    # maybe others like @reconciliation_hook

"""Workflow for SCCS-requested deposits.

The deposits managed by this workflow are requested by the Scholarly
Communication and Collection Strategy (SCCS) department
and are for submission to DSpace@MIT.
"""

workflow_name: str = "sccs"
metadata_mapping_path: str = "dsc/workflows/metadata_mapping/sccs.json"
6 changes: 2 additions & 4 deletions tests/conftest.py
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,6 @@ class TestWorkflow(Workflow):

workflow_name: str = "test"
submission_system: str = "Test@MIT"
email_recipients: tuple[str] = ("test@test.test",)
metadata_mapping_path: str = "tests/fixtures/test_metadata_mapping.json"

def item_metadata_iter(self):
Expand Down Expand Up @@ -56,10 +55,7 @@ class TestSimpleCSV(SimpleCSV):

workflow_name = "simple_csv"
submission_system: str = "Test@MIT"
email_recipients: tuple[str] = ("test@test.test",)
metadata_mapping_path: str = "tests/fixtures/test_metadata_mapping.json"
s3_bucket: str = "dsc"
output_queue: str = "mock-output_queue"


@pytest.fixture(autouse=True)
Expand All @@ -79,6 +75,7 @@ def base_workflow_instance(item_metadata, metadata_mapping, mocked_s3):
collection_handle="123.4/5678",
batch_id="batch-aaa",
email_recipients=["test@test.test"],
output_queue="mock-output_queue",
)


Expand All @@ -88,6 +85,7 @@ def simple_csv_workflow_instance(metadata_mapping):
collection_handle="123.4/5678",
batch_id="batch-aaa",
email_recipients=["test@test.test"],
output_queue="mock-output_queue",
)


Expand Down
Loading