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

#331 etags on uploads #577

Merged
merged 8 commits into from
Sep 13, 2016
Merged

Conversation

julianseeger
Copy link
Collaborator

@julianseeger julianseeger commented Aug 30, 2016


interface Hasher {
@Throws(IOException::class)
fun getMHash(file: Path): String
Copy link
Collaborator

Choose a reason for hiding this comment

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

get_M_Hash?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

the mhash predates this PR: https://github.com/Qabel/qabel-core/search?utf8=%E2%9C%93&q=mhash
It was an evolution from mtime to "still mtime but let's not call it time" ;)
But sure, this Interface doesn't need to know anything about that legacy

@Test
fun uploadsSuccessfulIfEtagIsUpToDate() {
val upload = writeBackend.upload("file", fakeDataStream())
writeBackend.upload("file", fakeDataStream("new"), upload.etag)
Copy link
Member

Choose a reason for hiding this comment

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

Assert that an etag actually exists and is not the empty string

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

done

}

companion object {
// Number of http connections to S3
Copy link
Member

Choose a reason for hiding this comment

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

not S3 anymore

@audax
Copy link
Member

audax commented Sep 13, 2016

You may resolve the remaining "issues" in a seperate pr ;)

@audax audax merged commit 2f409bb into Qabel:master Sep 13, 2016
// Number of http connections to S3
// The default was too low, 20 works. Further testing may be required
// to find the best amount of connections.
protected val CONNECTIONS = 50
Copy link
Collaborator

@enkore enkore Sep 13, 2016

Choose a reason for hiding this comment

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

using HTTP pipelining should reduce that number to essentially 1. (pipelining POSTs is safe for this application imho). create ticket?

https://hc.apache.org/httpcomponents-core-ga/httpcore-nio/examples/org/apache/http/examples/nio/PipeliningHttpClient.java

@audax
Copy link
Member

audax commented Sep 14, 2016

Create ticket.

import org.apache.http.client.methods.HttpPost
import org.apache.http.client.utils.DateUtils
import org.apache.http.entity.InputStreamEntity
import sun.plugin.dom.exception.InvalidStateException
Copy link
Member

Choose a reason for hiding this comment

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

wtf

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

autoimport o.O

Copy link
Collaborator

Choose a reason for hiding this comment

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

Should be IllegalStateException?

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.

reduce conflict window for uploads
3 participants