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

snappy-framed implementation #16

Closed
bokken opened this issue Mar 11, 2013 · 10 comments
Closed

snappy-framed implementation #16

bokken opened this issue Mar 11, 2013 · 10 comments

Comments

@bokken
Copy link
Contributor

bokken commented Mar 11, 2013

Has any thought been given to providing a x-snappy-framed implementation[1]?

I would be willing to provide an implementation if there is interest.

http://code.google.com/p/snappy/source/browse/trunk/format_description.txt

@electrum
Copy link
Collaborator

This is the correct link to the framing format: https://code.google.com/p/snappy/source/browse/trunk/framing_format.txt

We're actually the ones who proposed the original framing format that SnappyInputStream / SnappyOutputStream implements. Unfortunately, we haven't had time to update the code to match the current specification.

Contributions are welcome. I think we need to keep compatibility on the InputFormat side for users that have existing compressed data. The current code uses a header of "snappy" whereas the new version uses "sNaPpY", so it should be easy to tell the two formats apart.

@bokken
Copy link
Contributor Author

bokken commented Mar 12, 2013

I was definitely reading both of those docs and pasted the wrong link. I apologize for the confusion.

To maintain compatibility, would it be sufficient to have a static method, perhaps in the Snappy class, which marks/resets and reads the header bytes to determine which implementation of input stream to use?

Also, is there any value in giving the existing Snappy InputStream/OutputStream a coding name? As long as it is distinct from x-snappy-framed, it would make it possible to (at least through http) clearly differentiate which is being used.

@bokken
Copy link
Contributor Author

bokken commented Mar 16, 2013

I forked your repo and checked in my changes.

[1] - bokken@0d5a6ef

@bokken
Copy link
Contributor Author

bokken commented Apr 25, 2013

Is anyone with commit privs monitoring this? Is this project still being maintained?

@bokken
Copy link
Contributor Author

bokken commented Jan 16, 2015

Any chance of this getting incorporated?

@dain
Copy link
Owner

dain commented Jan 24, 2015

Can you add this license header to every file?

/*
 * Copyright (C) 2011 the original author or authors.
 * See the notice.md file distributed with this work for additional
 * information regarding copyright ownership.
 *
 * Licensed under the Apache License, Version 2.0 (the "License");
 * you may not use this file except in compliance with the License.
 * You may obtain a copy of the License at
 *
 *     http://www.apache.org/licenses/LICENSE-2.0
 *
 * Unless required by applicable law or agreed to in writing, software
 * distributed under the License is distributed on an "AS IS" BASIS,
 * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
 * See the License for the specific language governing permissions and
 * limitations under the License.
 */

Also we don't use @author tags as they are impossible to keep up to date as the code evolves. Would you be ok with removing them?

@dain
Copy link
Owner

dain commented Jan 24, 2015

Actually, I can make the changes if you say ok.

@bokken
Copy link
Contributor Author

bokken commented Jan 24, 2015

That is fine.
On Jan 24, 2015 2:26 PM, "Dain Sundstrom" notifications@github.com wrote:

Actually, I can make the changes if you say ok.


Reply to this email directly or view it on GitHub
#16 (comment).

@dain
Copy link
Owner

dain commented Jan 25, 2015

I cleaned up the commit a bit and merged it. Please, let me know if it is working for you and I'll do a release.

@dain dain closed this as completed Jan 25, 2015
@bokken
Copy link
Contributor Author

bokken commented Jan 26, 2015

I have tested it and everything appears to be good. I have added support to implement nio Channel implementations and utility methods for bulk transfers as part of a pull request.

#27

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

3 participants