You are viewing a plain text version of this content. The canonical link for it is here.
Posted to pr@jena.apache.org by GitBox <gi...@apache.org> on 2021/10/22 12:10:49 UTC
[GitHub] [jena] afs opened a new pull request #1089: JENA-2181: Placehold code for zst support
afs opened a new pull request #1089:
URL: https://github.com/apache/jena/pull/1089
Not active - Apache Commons Compress uses an external JNI library for decompression. It means having to ship multi-architecture binaries for packaged binaries (Jena download and Fuseki).
Other clean up.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: pr-unsubscribe@jena.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: pr-unsubscribe@jena.apache.org
For additional commands, e-mail: pr-help@jena.apache.org
[GitHub] [jena] rvesse commented on a change in pull request #1089: JENA-2181: Placeholder code for zst support
Posted by GitBox <gi...@apache.org>.
rvesse commented on a change in pull request #1089:
URL: https://github.com/apache/jena/pull/1089#discussion_r734631860
##########
File path: jena-base/src/main/java/org/apache/jena/atlas/io/IO.java
##########
@@ -87,9 +90,10 @@ static public InputStream openFileEx(String filename) throws IOException, FileNo
InputStream in = new FileInputStream(filename);
String ext = getExtension(filename);
switch ( ext ) {
- case "": return in;
+ case ext_none: return in;
case ext_gz: return new GZIPInputStream(in);
case ext_bz2: return new BZip2CompressorInputStream(in);
+ //case ext_zst: return new ZstdCompressorInputStream(in);
Review comment:
Could these be uncommented?
I understand that this requires a separate native library but if not present what is the behaviour here? Would the class instantiation just error out because it can't load the native library?
I guess what I'm asking is whether there is any harm in enabling this and just adding user documentation that says "If you want Zstd compression please install the relevant native library for your platform" and thus bypass any need for us to ship those libraries
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: pr-unsubscribe@jena.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: pr-unsubscribe@jena.apache.org
For additional commands, e-mail: pr-help@jena.apache.org
[GitHub] [jena] rvesse commented on a change in pull request #1089: JENA-2181: Placeholder code for zst support
Posted by GitBox <gi...@apache.org>.
rvesse commented on a change in pull request #1089:
URL: https://github.com/apache/jena/pull/1089#discussion_r735427720
##########
File path: jena-base/src/main/java/org/apache/jena/atlas/io/IO.java
##########
@@ -87,9 +90,10 @@ static public InputStream openFileEx(String filename) throws IOException, FileNo
InputStream in = new FileInputStream(filename);
String ext = getExtension(filename);
switch ( ext ) {
- case "": return in;
+ case ext_none: return in;
case ext_gz: return new GZIPInputStream(in);
case ext_bz2: return new BZip2CompressorInputStream(in);
+ //case ext_zst: return new ZstdCompressorInputStream(in);
Review comment:
> we have an obligation
Any Apache project, Jena included, is volunteer run and driven. Volunteers are inevitably a limited resource and there is a limit to how much the project can commit to maintain.
As @afs notes anything we do support, even as an optional component, places additional burden on the project.
```
In https://commons.apache.org/proper/commons-compress/ , they say zstd-jni is BSD licensed.
So according to https://www.apache.org/legal/resolved.html#category-a they can be included in the project,
```
Yes but including the `zstd-jni` library does not do anything in of itself, that's just a wrapper to the native OS libraries. The burden is still on the user to install the necessary native library to make it actually work
@afs Maybe a better path is to make compression support an extension point so this sort of thing could be dropped in by a 3rd party library?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: pr-unsubscribe@jena.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: pr-unsubscribe@jena.apache.org
For additional commands, e-mail: pr-help@jena.apache.org
[GitHub] [jena] afs commented on a change in pull request #1089: JENA-2181: Placeholder code for zst support
Posted by GitBox <gi...@apache.org>.
afs commented on a change in pull request #1089:
URL: https://github.com/apache/jena/pull/1089#discussion_r734694236
##########
File path: jena-base/src/main/java/org/apache/jena/atlas/io/IO.java
##########
@@ -87,9 +90,10 @@ static public InputStream openFileEx(String filename) throws IOException, FileNo
InputStream in = new FileInputStream(filename);
String ext = getExtension(filename);
switch ( ext ) {
- case "": return in;
+ case ext_none: return in;
case ext_gz: return new GZIPInputStream(in);
case ext_bz2: return new BZip2CompressorInputStream(in);
+ //case ext_zst: return new ZstdCompressorInputStream(in);
Review comment:
I'd rather not until that has been tested. I left the placeholder code in as I'd done the setup work.
Mentioning in the document means someone may well try and then we have an obligation.
The situation with Fuseki is complicated because of repacking jars.
Work on [a Kafka connector to Fuseki](https://github.com/Telicent-io/jena-fuseki-kafka) might get something done as Kafka seems to pull in ZStandard - I haven't investigated further.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: pr-unsubscribe@jena.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: pr-unsubscribe@jena.apache.org
For additional commands, e-mail: pr-help@jena.apache.org
[GitHub] [jena] afs commented on pull request #1089: JENA-2181: Placeholder code for zst support
Posted by GitBox <gi...@apache.org>.
afs commented on pull request #1089:
URL: https://github.com/apache/jena/pull/1089#issuecomment-953992540
PR reset to include only the IO.java changes.
Other maintenance changes removed and put on a separate PR.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: pr-unsubscribe@jena.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: pr-unsubscribe@jena.apache.org
For additional commands, e-mail: pr-help@jena.apache.org
[GitHub] [jena] ieugen commented on a change in pull request #1089: JENA-2181: Placeholder code for zst support
Posted by GitBox <gi...@apache.org>.
ieugen commented on a change in pull request #1089:
URL: https://github.com/apache/jena/pull/1089#discussion_r735447180
##########
File path: jena-base/src/main/java/org/apache/jena/atlas/io/IO.java
##########
@@ -87,9 +90,10 @@ static public InputStream openFileEx(String filename) throws IOException, FileNo
InputStream in = new FileInputStream(filename);
String ext = getExtension(filename);
switch ( ext ) {
- case "": return in;
+ case ext_none: return in;
case ext_gz: return new GZIPInputStream(in);
case ext_bz2: return new BZip2CompressorInputStream(in);
+ //case ext_zst: return new ZstdCompressorInputStream(in);
Review comment:
Thanks @rvesse ,
> @afs Maybe a better path is to make compression support an extension point so this sort of thing could be dropped in by a 3rd party library?
I'm perfectly fine with the app failing with a message saying: You need to have this library and this library installed to use zst or a link to the docs regarding zstandard.
Having to install a library to have improved performance and reduced time is something that only some people will care about (who have large amounts of data). The others will probably continue using gzip.
Having the option commented out means we would have to maintain and build a copy of fuseki - so will everyone else with similar needs.
Having it be on and OPT-IN (by adding libraries) means it's a lot easier to work with.
For one of our datasets using zstandard reduces compression time with 5min and datasize with 10GB (which is another ~10 min of copying time to wait) and we have several of these.
p.s. I'm also an Apache committer :)
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: pr-unsubscribe@jena.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: pr-unsubscribe@jena.apache.org
For additional commands, e-mail: pr-help@jena.apache.org
[GitHub] [jena] afs closed pull request #1089: JENA-2181: Placeholder code for zst support
Posted by GitBox <gi...@apache.org>.
afs closed pull request #1089:
URL: https://github.com/apache/jena/pull/1089
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: pr-unsubscribe@jena.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: pr-unsubscribe@jena.apache.org
For additional commands, e-mail: pr-help@jena.apache.org
[GitHub] [jena] ieugen commented on a change in pull request #1089: JENA-2181: Placeholder code for zst support
Posted by GitBox <gi...@apache.org>.
ieugen commented on a change in pull request #1089:
URL: https://github.com/apache/jena/pull/1089#discussion_r738444041
##########
File path: jena-base/src/main/java/org/apache/jena/atlas/io/IO.java
##########
@@ -87,9 +90,10 @@ static public InputStream openFileEx(String filename) throws IOException, FileNo
InputStream in = new FileInputStream(filename);
String ext = getExtension(filename);
switch ( ext ) {
- case "": return in;
+ case ext_none: return in;
case ext_gz: return new GZIPInputStream(in);
case ext_bz2: return new BZip2CompressorInputStream(in);
+ //case ext_zst: return new ZstdCompressorInputStream(in);
Review comment:
The doc changes are nice regardless of what happens with this PR.
That being said there is a usecase (I'm told by team members) where Fuseki load compressed NQ files from URL's .
This is driven by API/ UI ?! and in this case without native zst support it won't work.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: pr-unsubscribe@jena.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: pr-unsubscribe@jena.apache.org
For additional commands, e-mail: pr-help@jena.apache.org
[GitHub] [jena] afs commented on pull request #1089: JENA-2181: Placeholder code for zst support
Posted by GitBox <gi...@apache.org>.
afs commented on pull request #1089:
URL: https://github.com/apache/jena/pull/1089#issuecomment-961943027
Closing - this PR isn't going anywhere.
If we are going to do anything, then let's do it comprehensively.
1. Have a registry of addition extension to decompressor.
2. In places that access a filename, consult the registry and add a wrapper.
There can be centralized in `IO`.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: pr-unsubscribe@jena.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: pr-unsubscribe@jena.apache.org
For additional commands, e-mail: pr-help@jena.apache.org
[GitHub] [jena] ieugen commented on a change in pull request #1089: JENA-2181: Placeholder code for zst support
Posted by GitBox <gi...@apache.org>.
ieugen commented on a change in pull request #1089:
URL: https://github.com/apache/jena/pull/1089#discussion_r735181323
##########
File path: jena-base/src/main/java/org/apache/jena/atlas/io/IO.java
##########
@@ -87,9 +90,10 @@ static public InputStream openFileEx(String filename) throws IOException, FileNo
InputStream in = new FileInputStream(filename);
String ext = getExtension(filename);
switch ( ext ) {
- case "": return in;
+ case ext_none: return in;
case ext_gz: return new GZIPInputStream(in);
case ext_bz2: return new BZip2CompressorInputStream(in);
+ //case ext_zst: return new ZstdCompressorInputStream(in);
Review comment:
I also think that mentioning in the docs is good enough.
We have some use cases where I think ZST will make a difference and building a docker image with Fuseki + the platform libs for zst will fork very nicely for us.
> Mentioning in the document means someone may well try and then we have an obligation.
Can you please explain more about "we have an obligation" ?
To my knowledge, the ASF cares only about source code and binaries are provided as a convenience for users.
https://www.apache.org/legal/release-policy.html#compiled-packages
> COMPILED PACKAGES
The Apache Software Foundation produces open source software. All releases are in the form of the source materials needed to make changes to the software being released.
As a convenience to users that might not have the appropriate tools to build a compiled version of the source, binary/bytecode packages MAY be distributed alongside official Apache releases. In all such cases, the binary/bytecode package MUST have the same version number as the source release and MUST only add binary/bytecode files that are the result of compiling that version of the source code release and its dependencies.
In https://commons.apache.org/proper/commons-compress/ , they say zstd-jni is BSD licensed.
So according to https://www.apache.org/legal/resolved.html#category-a they can be included in the project,
Having a notice is perfectly acceptable IMO .
I don't think there is any legal issue here and we can go ahead with the patch to allow Zstandard.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: pr-unsubscribe@jena.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: pr-unsubscribe@jena.apache.org
For additional commands, e-mail: pr-help@jena.apache.org
[GitHub] [jena] afs commented on a change in pull request #1089: JENA-2181: Placeholder code for zst support
Posted by GitBox <gi...@apache.org>.
afs commented on a change in pull request #1089:
URL: https://github.com/apache/jena/pull/1089#discussion_r735504494
##########
File path: jena-base/src/main/java/org/apache/jena/atlas/io/IO.java
##########
@@ -87,9 +90,10 @@ static public InputStream openFileEx(String filename) throws IOException, FileNo
InputStream in = new FileInputStream(filename);
String ext = getExtension(filename);
switch ( ext ) {
- case "": return in;
+ case ext_none: return in;
case ext_gz: return new GZIPInputStream(in);
case ext_bz2: return new BZip2CompressorInputStream(in);
+ //case ext_zst: return new ZstdCompressorInputStream(in);
Review comment:
Let's try to focus this debate and base it on real utility offset against costs.
Jena can already process zst files so we need to be clearer about what problem is actually being addressed here.
```
InputStream input = new ZstdCompressorInputStream(....);
RDFParser.source(input).lang(Lang.NQ).parse(graph);
```
or from the command line:
```
zstd -d < FILE.nq.zst | riot --syntax NQ ...
```
gz is common compression format used in published RDF datasets.
bz2 is included because wikidata publishes bz2 files.
zst?
> Can you please explain more about "we have an obligation" ?
If it is in the documentation, it is reasonable to expect it to work. Our users@ list answers questions - almost every question gets a reply. Our community.
No one mentioned legal - of course, it is possible but the NOTICE is mandatory for redistribution of BSD-licensed code (here - two entries, one for zstd-jni one for zStandard itself). If zstd-jni is in a Fuseki server jar or war file, this project is redistributing that library and we do not know on what architecture it is running on.
Jena users are not all Java programmers.
> To my knowledge, the ASF cares only about source code and binaries are provided as a convenience for users.
That is out of context and plain wrong.
Convenience binaries must also meet ASF licensing governance.
Convenience binaries for a release are a product of a project and covered by the legal cover granted by being voted on.
Source is the output of a project but convenience binaries for Java-based projects are often the primary means of consumption. This project cares. Each Apache project is an independent community.
Very few people using Jena will rebuild from source. This is particularly true for Fuseki.
>We have some use cases where I think ZST will make a difference and building a docker image with Fuseki + the platform libs for zst will fork very nicely for us.
Jena already can parse zstd-compressed files so we need to be clearer about what problem is actually being addressed here.
For Fuseki, content negotiation happens. Where does zst fit into that?
@ieugen -- what is are the use cases? How do you expect to use the files with Fuseki?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: pr-unsubscribe@jena.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: pr-unsubscribe@jena.apache.org
For additional commands, e-mail: pr-help@jena.apache.org
[GitHub] [jena] afs commented on a change in pull request #1089: JENA-2181: Placeholder code for zst support
Posted by GitBox <gi...@apache.org>.
afs commented on a change in pull request #1089:
URL: https://github.com/apache/jena/pull/1089#discussion_r735504494
##########
File path: jena-base/src/main/java/org/apache/jena/atlas/io/IO.java
##########
@@ -87,9 +90,10 @@ static public InputStream openFileEx(String filename) throws IOException, FileNo
InputStream in = new FileInputStream(filename);
String ext = getExtension(filename);
switch ( ext ) {
- case "": return in;
+ case ext_none: return in;
case ext_gz: return new GZIPInputStream(in);
case ext_bz2: return new BZip2CompressorInputStream(in);
+ //case ext_zst: return new ZstdCompressorInputStream(in);
Review comment:
Let's try to focus this debate and base it on real utility offset against costs.
Jena can already process zst files so we need to be clearer about what problem is actually being addressed here.
```
InputStream input = new ZstdCompressorInputStream(....);
RDFParser.source(input).lang(Lang.NQ).parse(graph);
```
or from the command line:
```
zstd -d < FILE.nq.zst | riot --syntax NQ ...
```
gz is common compression format used in published RDF datasets.
bz2 is included because wikidata publishes bz2 files.
zst?
> Can you please explain more about "we have an obligation" ?
If it is in the documentation, it is reasonable to expect it to work. Our users@ list answers questions - almost every question gets a reply. Our community.
No one mentioned legal - of course, it is possible but the NOTICE is mandatory for redistribution of BSD-licensed code (here - two entries, one for zstd-jni one for zStandard itself). If zstd-jni is in a Fuseki server jar or war file, this project is redistributing that library and we do not know on what architecture it is running on.
Jena users are not all Java programmers.
> To my knowledge, the ASF cares only about source code and binaries are provided as a convenience for users.
That is out of context and plain wrong.
Convenience binaries must also meet ASF licensing governance.
Convenience binaries for a release are a product of a project and covered by the legal cover granted by being voted on.
Source is the output of a project but convenience binaries for Java-based projects are often the primary means of consumption. This project cares. Each Apache project is an independent community.
Very few people using Jena will rebuild from source. This is particularly true for Fuseki.
>We have some use cases where I think ZST will make a difference and building a docker image with Fuseki + the platform libs for zst will fork very nicely for us.
Jena already can parse zstd-compressed files so we need to be clearer about what problem is actually being addressed here.
For Fuseki, content negotiation happens. Where does zst fit into that?
@ieugen -- what are the use cases? How do you expect to use the files with Fuseki?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: pr-unsubscribe@jena.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: pr-unsubscribe@jena.apache.org
For additional commands, e-mail: pr-help@jena.apache.org
[GitHub] [jena] rvesse commented on a change in pull request #1089: JENA-2181: Placeholder code for zst support
Posted by GitBox <gi...@apache.org>.
rvesse commented on a change in pull request #1089:
URL: https://github.com/apache/jena/pull/1089#discussion_r735427720
##########
File path: jena-base/src/main/java/org/apache/jena/atlas/io/IO.java
##########
@@ -87,9 +90,10 @@ static public InputStream openFileEx(String filename) throws IOException, FileNo
InputStream in = new FileInputStream(filename);
String ext = getExtension(filename);
switch ( ext ) {
- case "": return in;
+ case ext_none: return in;
case ext_gz: return new GZIPInputStream(in);
case ext_bz2: return new BZip2CompressorInputStream(in);
+ //case ext_zst: return new ZstdCompressorInputStream(in);
Review comment:
> we have an obligation
Any Apache project, Jena included, is volunteer run and driven. Volunteers are inevitably a limited resource and there is a limit to how much the project can commit to maintain.
As @afs notes anything we do support, even as an optional component, places additional burden on the project. Anything we say we support people will use, even if they don't tell us, **but** they often complain loudly as and when we decide we can't support it anymore and try and deprecate it.
```
In https://commons.apache.org/proper/commons-compress/ , they say zstd-jni is BSD licensed.
So according to https://www.apache.org/legal/resolved.html#category-a they can be included in the project,
```
Yes but including the `zstd-jni` library does not do anything in of itself, that's just a wrapper to the native OS libraries. The burden is still on the user to install the necessary native library to make it actually work
@afs Maybe a better path is to make compression support an extension point so this sort of thing could be dropped in by a 3rd party library?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: pr-unsubscribe@jena.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: pr-unsubscribe@jena.apache.org
For additional commands, e-mail: pr-help@jena.apache.org
[GitHub] [jena] ieugen commented on a change in pull request #1089: JENA-2181: Placeholder code for zst support
Posted by GitBox <gi...@apache.org>.
ieugen commented on a change in pull request #1089:
URL: https://github.com/apache/jena/pull/1089#discussion_r735181350
##########
File path: jena-base/src/main/java/org/apache/jena/atlas/io/IO.java
##########
@@ -87,9 +90,10 @@ static public InputStream openFileEx(String filename) throws IOException, FileNo
InputStream in = new FileInputStream(filename);
String ext = getExtension(filename);
switch ( ext ) {
- case "": return in;
+ case ext_none: return in;
case ext_gz: return new GZIPInputStream(in);
case ext_bz2: return new BZip2CompressorInputStream(in);
+ //case ext_zst: return new ZstdCompressorInputStream(in);
Review comment:
BTW: Thanks for working on this.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: pr-unsubscribe@jena.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: pr-unsubscribe@jena.apache.org
For additional commands, e-mail: pr-help@jena.apache.org
[GitHub] [jena] rvesse commented on a change in pull request #1089: JENA-2181: Placeholder code for zst support
Posted by GitBox <gi...@apache.org>.
rvesse commented on a change in pull request #1089:
URL: https://github.com/apache/jena/pull/1089#discussion_r738164614
##########
File path: jena-base/src/main/java/org/apache/jena/atlas/io/IO.java
##########
@@ -87,9 +90,10 @@ static public InputStream openFileEx(String filename) throws IOException, FileNo
InputStream in = new FileInputStream(filename);
String ext = getExtension(filename);
switch ( ext ) {
- case "": return in;
+ case ext_none: return in;
case ext_gz: return new GZIPInputStream(in);
case ext_bz2: return new BZip2CompressorInputStream(in);
+ //case ext_zst: return new ZstdCompressorInputStream(in);
Review comment:
See apache/jena-site#72 for possible documentation changes
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: pr-unsubscribe@jena.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: pr-unsubscribe@jena.apache.org
For additional commands, e-mail: pr-help@jena.apache.org
[GitHub] [jena] ieugen commented on a change in pull request #1089: JENA-2181: Placeholder code for zst support
Posted by GitBox <gi...@apache.org>.
ieugen commented on a change in pull request #1089:
URL: https://github.com/apache/jena/pull/1089#discussion_r739123668
##########
File path: jena-base/src/main/java/org/apache/jena/atlas/io/IO.java
##########
@@ -87,9 +90,10 @@ static public InputStream openFileEx(String filename) throws IOException, FileNo
InputStream in = new FileInputStream(filename);
String ext = getExtension(filename);
switch ( ext ) {
- case "": return in;
+ case ext_none: return in;
case ext_gz: return new GZIPInputStream(in);
case ext_bz2: return new BZip2CompressorInputStream(in);
+ //case ext_zst: return new ZstdCompressorInputStream(in);
Review comment:
Thanks @afs
We build our own docker images from the published tar.gz binary releases of Apache + maybe some other scripts that we want to drop since fuseki added support for those operations.
I think @peterbednar can help answer questions about:
> How would you be sending the file to Fuseki? File upload or Graph Store Protocol?
> What HTTP client software are you using?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: pr-unsubscribe@jena.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: pr-unsubscribe@jena.apache.org
For additional commands, e-mail: pr-help@jena.apache.org
[GitHub] [jena] ieugen commented on a change in pull request #1089: JENA-2181: Placeholder code for zst support
Posted by GitBox <gi...@apache.org>.
ieugen commented on a change in pull request #1089:
URL: https://github.com/apache/jena/pull/1089#discussion_r735181323
##########
File path: jena-base/src/main/java/org/apache/jena/atlas/io/IO.java
##########
@@ -87,9 +90,10 @@ static public InputStream openFileEx(String filename) throws IOException, FileNo
InputStream in = new FileInputStream(filename);
String ext = getExtension(filename);
switch ( ext ) {
- case "": return in;
+ case ext_none: return in;
case ext_gz: return new GZIPInputStream(in);
case ext_bz2: return new BZip2CompressorInputStream(in);
+ //case ext_zst: return new ZstdCompressorInputStream(in);
Review comment:
I also think that mentioning in the docs is good enough.
We have some use cases where I think ZST will make a difference and building a docker image with Fuseki + the platform libs for zst will fork very nicely for us.
> Mentioning in the document means someone may well try and then we have an obligation.
Can you please explain more about "we have an obligation" ?
To my knowledge, the ASF cares only about source code and binaries are provided as a convenience for users.
https://www.apache.org/legal/release-policy.html#compiled-packages
> COMPILED PACKAGES
The Apache Software Foundation produces open source software. All releases are in the form of the source materials needed to make changes to the software being released.
As a convenience to users that might not have the appropriate tools to build a compiled version of the source, binary/bytecode packages MAY be distributed alongside official Apache releases. In all such cases, the binary/bytecode package MUST have the same version number as the source release and MUST only add binary/bytecode files that are the result of compiling that version of the source code release and its dependencies.
In https://commons.apache.org/proper/commons-compress/ , they say zstd-jni is BSD licensed.
So according to https://www.apache.org/legal/resolved.html#category-a they can be included in the project,
There is also https://www.apache.org/legal/resolved.html#binary-only-inclusion-condition for Category B licenses ( BSD is Category A)
Having a notice is perfectly acceptable IMO .
I don't think there is any legal issue here and we can go ahead with the patch to allow Zstandard.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: pr-unsubscribe@jena.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: pr-unsubscribe@jena.apache.org
For additional commands, e-mail: pr-help@jena.apache.org
[GitHub] [jena] afs closed pull request #1089: JENA-2181: Placeholder code for zst support
Posted by GitBox <gi...@apache.org>.
afs closed pull request #1089:
URL: https://github.com/apache/jena/pull/1089
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: pr-unsubscribe@jena.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: pr-unsubscribe@jena.apache.org
For additional commands, e-mail: pr-help@jena.apache.org
[GitHub] [jena] rvesse commented on a change in pull request #1089: JENA-2181: Placeholder code for zst support
Posted by GitBox <gi...@apache.org>.
rvesse commented on a change in pull request #1089:
URL: https://github.com/apache/jena/pull/1089#discussion_r738149627
##########
File path: jena-base/src/main/java/org/apache/jena/atlas/io/IO.java
##########
@@ -87,9 +90,10 @@ static public InputStream openFileEx(String filename) throws IOException, FileNo
InputStream in = new FileInputStream(filename);
String ext = getExtension(filename);
switch ( ext ) {
- case "": return in;
+ case ext_none: return in;
case ext_gz: return new GZIPInputStream(in);
case ext_bz2: return new BZip2CompressorInputStream(in);
+ //case ext_zst: return new ZstdCompressorInputStream(in);
Review comment:
> Let's try to focus this debate and base it on real utility offset against costs.
>
> Jena can already process zst files so we need to be clearer about what problem is actually being addressed here.
>
> ```
> InputStream input = new ZstdCompressorInputStream(....);
> RDFParser.source(input).lang(Lang.NQ).parse(graph);
> ```
>
> or from the command line:
>
> ```
> zstd -d < FILE.nq.zst | riot --syntax NQ ...
> ```
Given this is the case rather than this PR would it be better just to have a new section in the docs that describes how to use other compression formats?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: pr-unsubscribe@jena.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: pr-unsubscribe@jena.apache.org
For additional commands, e-mail: pr-help@jena.apache.org
[GitHub] [jena] afs commented on a change in pull request #1089: JENA-2181: Placeholder code for zst support
Posted by GitBox <gi...@apache.org>.
afs commented on a change in pull request #1089:
URL: https://github.com/apache/jena/pull/1089#discussion_r738514229
##########
File path: jena-base/src/main/java/org/apache/jena/atlas/io/IO.java
##########
@@ -87,9 +90,10 @@ static public InputStream openFileEx(String filename) throws IOException, FileNo
InputStream in = new FileInputStream(filename);
String ext = getExtension(filename);
switch ( ext ) {
- case "": return in;
+ case ext_none: return in;
case ext_gz: return new GZIPInputStream(in);
case ext_bz2: return new BZip2CompressorInputStream(in);
+ //case ext_zst: return new ZstdCompressorInputStream(in);
Review comment:
That is a different part of the system. It would have been helpful to have that information from the start.
This is exactly why I was cautions about including it - putting increasing specialised use cases into the general code base creates problems and costs. If the original JIRA were implemented, it would make no difference to your usage.
Fuseki does not upload from URLs. There would be significant security challenges to carefully address to pull data.
Which packaging of Fuseki are you using?
How would you be sending the file to Fuseki? File upload or Graph Store Protocol?
What HTTP client software are you using?
And as above in the thread - how are you getting the JNI code into Fuseki?
Or are you using HTTP `Content-Encoding`?
Fuseki can apply `Content-Encoding` in file upload function, GSP and extended GSP. But see https://developer.mozilla.org/en-US/docs/Web/HTTP/Headers/Content-Encoding
and note that only a few compression formats are well known.
If you are using `Transfer-Encoding`, again only gzip and deflate are generally provided and it is unlikely to help you because proxies are involved .
ZStandard may be fast but when I tried it, did not produce that much smaller files.
The speed of loading any significant amount of data into a database is not strongly influenced by compression. The network is faster delivering data than the loader is capable of incrementally loading. Sending uncompressed is just as fast, if not faster.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: pr-unsubscribe@jena.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: pr-unsubscribe@jena.apache.org
For additional commands, e-mail: pr-help@jena.apache.org
[GitHub] [jena] ieugen commented on a change in pull request #1089: JENA-2181: Placeholder code for zst support
Posted by GitBox <gi...@apache.org>.
ieugen commented on a change in pull request #1089:
URL: https://github.com/apache/jena/pull/1089#discussion_r735774389
##########
File path: jena-base/src/main/java/org/apache/jena/atlas/io/IO.java
##########
@@ -87,9 +90,10 @@ static public InputStream openFileEx(String filename) throws IOException, FileNo
InputStream in = new FileInputStream(filename);
String ext = getExtension(filename);
switch ( ext ) {
- case "": return in;
+ case ext_none: return in;
case ext_gz: return new GZIPInputStream(in);
case ext_bz2: return new BZip2CompressorInputStream(in);
+ //case ext_zst: return new ZstdCompressorInputStream(in);
Review comment:
Hi, thanks @afs your comment got me thinking.
Please let me take a look and come back on this.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: pr-unsubscribe@jena.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: pr-unsubscribe@jena.apache.org
For additional commands, e-mail: pr-help@jena.apache.org
[GitHub] [jena] afs commented on pull request #1089: JENA-2181: Placeholder code for zst support
Posted by GitBox <gi...@apache.org>.
afs commented on pull request #1089:
URL: https://github.com/apache/jena/pull/1089#issuecomment-961943027
Closing - this PR isn't going anywhere.
If we are going to do anything, then let's do it comprehensively.
1. Have a registry of addition extension to decompressor.
2. In places that access a filename, consult the registry and add a wrapper.
There can be centralized in `IO`.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: pr-unsubscribe@jena.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: pr-unsubscribe@jena.apache.org
For additional commands, e-mail: pr-help@jena.apache.org
[GitHub] [jena] afs commented on pull request #1089: JENA-2181: Placeholder code for zst support
Posted by GitBox <gi...@apache.org>.
afs commented on pull request #1089:
URL: https://github.com/apache/jena/pull/1089#issuecomment-961943027
Closing - this PR isn't going anywhere.
If we are going to do anything, then let's do it comprehensively.
1. Have a registry of addition extension to decompressor.
2. In places that access a filename, consult the registry and add a wrapper.
There can be centralized in `IO`.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: pr-unsubscribe@jena.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: pr-unsubscribe@jena.apache.org
For additional commands, e-mail: pr-help@jena.apache.org
[GitHub] [jena] afs closed pull request #1089: JENA-2181: Placeholder code for zst support
Posted by GitBox <gi...@apache.org>.
afs closed pull request #1089:
URL: https://github.com/apache/jena/pull/1089
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: pr-unsubscribe@jena.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: pr-unsubscribe@jena.apache.org
For additional commands, e-mail: pr-help@jena.apache.org