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

Don't take snapshot on asset upload before workflow #5247

Merged
merged 3 commits into from Sep 29, 2023

Conversation

KatrinIhler
Copy link
Member

The workflow already takes a snapshot, and otherwise we can't figure out which assets are new in the workflow (which I need later).

This also actually makes the code a bit simpler.

The workflow already takes a snapshot, and otherwise we can't figure out
which assets are new.
@KatrinIhler KatrinIhler added ELAN Pull requests originating from ELAN e.V. java Pull requests that update Java code labels Sep 12, 2023
Copy link
Member

@Arnei Arnei left a comment

Choose a reason for hiding this comment

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

Looks good and works as expected.

Since taking a snapshot at the beginning of the asset upload workflow still seems to be a good idea, would it make sense to add a comment on that to etc/event.upload.asset.options.propertires (or maybe in the docs, wherever it would make more sense)?

Also, the comment for startAddAssetWorkflow(JSONObject metadataJson, MediaPackage mediaPackage) talks at length about a race condition due to taking a snapshot prior to starting a workflow. Since that's not happening anymore, I suppose the potential race condition can't happen anymore either?

As we no longer take a snapshot before starting the workflow, and the
workflow service checks for running workflows immediately before
starting the workflow, this no longer applies.
@KatrinIhler
Copy link
Member Author

@Arnei As we talked about, I will not document the need for a snapshot, as that's fairly obvious, but I did remove the comment about the race condition since it no longer applies.

@KatrinIhler
Copy link
Member Author

Approved and discussed, gonna merge this.

@KatrinIhler KatrinIhler merged commit e419dd6 into opencast:develop Sep 29, 2023
4 checks passed
KatrinIhler added a commit that referenced this pull request Sep 29, 2023
In accordance with the recent changes to subtitles in Opencast, captions
need special handling when they're uploaded to an existing event, e.g.
they must be cut. This extends the existing workflow to handle this use
case.

**Attention:** This change depends on #5243 and #5247 to work and should
not be merged separately!

Also contains a minor fix in another workflow.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
ELAN Pull requests originating from ELAN e.V. java Pull requests that update Java code
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants