Skip to content

Conversation

@dchandekstark
Copy link
Contributor

No description provided.

@dchandekstark
Copy link
Contributor Author

Review please @jcoyne @jeremyf @barmintor

Copy link
Contributor

Choose a reason for hiding this comment

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

entity_size is a more idiomatic name.

Copy link
Contributor

Choose a reason for hiding this comment

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

or entry_size_for implying that there is a parameter?

@jeremyf
Copy link
Contributor

jeremyf commented Sep 15, 2014

I don't have enough experience with Rubydora regarding this.

From the Hydra Committers call on 2014-09-15

The stream method in Rubydora class, at the time written, did not take into account providing support for external data streams. When it attempts to deal with range requests, for external datastreams this is always zero. This patch handles the case of external datastreams. Hydra downloads uses the stream method so this patch addresses those changes.
-- @dchandekstark

The `stream` method relied on the `dsSize` attribute of the datastream to report
the content length; however, a Fedora 3 repo always reports `dsSize` 0 for
external datastreams. This patch permist the existing method to work with
external datastreams while preserving the original behavior for other type
(managed and inline).  Since Hydra::Controller::DownloadBehavior relies on this
method, the patch allows that module to function on external datastreams without
modification.
@dchandekstark
Copy link
Contributor Author

Changed private method name per @jcoyne and added detailed explanation to commit message per @jeremyf.

jeremyf added a commit that referenced this pull request Sep 15, 2014
Patched Datastream#stream to work with controlGroup E (fixes #88)
@jeremyf jeremyf merged commit 6d5f83a into samvera-deprecated:master Sep 15, 2014
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

Successfully merging this pull request may close these issues.

3 participants