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

make active encode work in hydra-derivatives #1785

Closed
3 of 4 tasks
davidschober opened this issue Mar 23, 2017 · 20 comments
Closed
3 of 4 tasks

make active encode work in hydra-derivatives #1785

davidschober opened this issue Mar 23, 2017 · 20 comments
Assignees
Milestone

Comments

@davidschober
Copy link

davidschober commented Mar 23, 2017

New branch started to work from:

https://github.com/projecthydra/hydra-derivatives/tree/active_encode_dev_con

Done looks like

  • create derivatives using active encode within hydra derivatives.
  • wait until encoding is complete and provide feedback if it failed (Follow patterns in other processors)
  • Possibly make a new output file service or maybe reuse the existing one (I'm not sure if the IODecorator can take a url).
  • Write tests and docs, submit PR.
@davidschober
Copy link
Author

This is not dependent upon the IIIF-AV spec. The spec only requires high, med, low. which is arbitrary.

@joncameron
Copy link
Contributor

"Make it so."

@val99erie val99erie self-assigned this Mar 28, 2017
@val99erie val99erie assigned cjcolvar and unassigned val99erie Mar 28, 2017
@little9
Copy link

little9 commented Mar 31, 2017

This will need to broken up so that @val99erie and I can help out

@cjcolvar
Copy link
Member

cjcolvar commented Apr 3, 2017

I have a work in progress branch here: https://github.com/projecthydra/hydra-derivatives/tree/active_encode_dev_con
I think next steps might be to wait until the encoding is complete in order to act more like the other processors and to provide feedback if it failed. Also if it waits then we could make a new output file service or maybe reuse the existing one (I'm not sure if the IODecorator can take a url). Last but not least we'll also need tests and documentation before submitting a PR. So it seems like we have 3-4 tasks that could be broken out.

@davidschober
Copy link
Author

Thanks! @csyversen do you wan too take a peak at this?

@davidschober
Copy link
Author

@little9 and @val99erie I broke out @cjcolvar tasks above. We can break these into new issues if it makes more sense. I'm a little unclear on the scope of each issue. Let me know.

@val99erie
Copy link

I have a few thoughts & questions:

  • It looks like the encoding job in Avalon polls for the status every 10 seconds. Is that how we intend to do it in hydra-derivtives too: just keep polling until it's done? 10 seconds seems like a good default, but the poll time should probably be configurable (within reason - I assume if you poll too often, you'll be DOS-attacking your own process).

  • Is is safe to assume/encourage that the encoding work will always be wrapped in a background job (or some other process that's allowed to take as long as it wants)? Do we need to handle timeouts, or can we literally just run until we get to a completion state?

@csyversen
Copy link
Contributor

this is out of scope at the moment, but it would be awesome to eventually write up hydra_derivatives to support rails 5's actioncable and do away with polling all together. This seems like a great use case for it.

( @val99erie i'm just writing down my thoughts right now, i don't expect us to move on this right now)

@davidschober
Copy link
Author

davidschober commented Apr 4, 2017 via email

@val99erie
Copy link

Question:

I added some code to handle the case where active_encode reports that the encoding has failed, but there is another edge case we should think about: active_encode has a :cancelled state.

How should we handle the case where active_encode reports that the job has been cancelled?
Some options:

  • Detect the :cancelled state and raise an exception with a message that it was cancelled so that there will be a corresponding failed background job with the cancel message.

  • Silently ignore :cancelled and treat it the same as :completed.

  • Make the behavior configurable (that is more complicated and could get ugly, depending on how many config cases we want to handle).

Probably the least surprising behavior to the end user would be to raise an exception with a cancelled message. That's what I would vote for unless there is a good reason to choose something else.

@bkeese
Copy link
Contributor

bkeese commented Apr 5, 2017

I second your vote.

@mcwhitaker
Copy link
Contributor

That is 3 votes now.

@davidschober
Copy link
Author

fourth if you need it. I believe that's quorum.

@val99erie
Copy link

Ok, I'll work on that code tomorrow. It should be reasonably easy to change the code in the future if we change our minds about what the behavior should be.

@cjcolvar
Copy link
Member

#1827 seems to be close to a duplicate of this story. Should we refine this story and close #1827 or close this one and copy over relevant pieces to #1827?

val99erie added a commit to samvera/hydra-derivatives that referenced this issue Apr 14, 2017
@val99erie
Copy link

val99erie commented Apr 14, 2017

Here's what we have left to do, as far as I know:

  • There are several TODOs in the code, such as configuring sleep time, handling timeouts, etc.
  • We need to figure out what happens once the encoding finishes successfully, do the FileSet and/or File record in fedora need to be updated with the URIs for the transcoded files? Or is there any other metadata we need to update?
  • We need to document how to use AE within HD (I have been keeping notes about that, so we can just copy those notes over once the code is finished)

@cjcolvar
Copy link
Member

Maybe this isn't needed in Proof of Concept, but I think we also need to have the ability to configure the subclass of ActiveEncode::Base to use within HD.

@val99erie
Copy link

@cjcolvar Yeah, that seems like a good idea not to hard-code the class.

cjcolvar added a commit to samvera/hydra-derivatives that referenced this issue Apr 18, 2017
Configure the poll time for ActiveEncode. Part of story avalonmediasystem/avalon#1785
val99erie added a commit to samvera/hydra-derivatives that referenced this issue Apr 18, 2017
@val99erie
Copy link

What to do about supplemental files like thumbnails or subtitles?

One other thing that we need to think about (perhaps in a separate story) is whether or not supplemental files would be stored externally or if we would need to download them to whereever fedora is storing its files. For example, if you create a thumbnail with Elastic Transcoder, would you serve the thumbnail from s3, or locally?

@val99erie
Copy link

After discussion in the slack channel, it seems that we should probably leave it up to the implementer to make sure all the files are in the correct place to serve from. Trying to do anything else would be a large scope creep for this story.

cjcolvar added a commit to samvera/hydra-derivatives that referenced this issue Apr 25, 2017
Add documentation about how to use Amazon encoding service. Part of story avalonmediasystem/avalon#1785
val99erie pushed a commit to samvera/hydra-derivatives that referenced this issue Apr 28, 2017
@davidschober davidschober modified the milestone: Sprint 125 May 5, 2017
@davidschober davidschober modified the milestone: Sprint 125 May 5, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

8 participants