You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@pekko.apache.org by "jypma (via GitHub)" <gi...@apache.org> on 2025/03/11 07:53:47 UTC
[PR] Add size and date information to ZipArchiveMetadata [pekko-connectors]
jypma opened a new pull request, #1002:
URL: https://github.com/apache/pekko-connectors/pull/1002
This brings the functionality closer to TarArchiveMetadata that already has these. Size and date information can be very valuable when unzipping files, to check if this entry has already been unzipped.
--
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: notifications-unsubscribe@pekko.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@pekko.apache.org
For additional commands, e-mail: notifications-help@pekko.apache.org
Re: [PR] FILE: Add size and date information to ZipArchiveMetadata [pekko-connectors]
Posted by "pjfanning (via GitHub)" <gi...@apache.org>.
pjfanning commented on code in PR #1002:
URL: https://github.com/apache/pekko-connectors/pull/1002#discussion_r1988801098
##########
file/src/main/scala/org/apache/pekko/stream/connectors/file/model.scala:
##########
@@ -25,11 +25,16 @@ object ArchiveMetadata {
def create(filePath: String): ArchiveMetadata = new ArchiveMetadata(filePath)
}
-final case class ZipArchiveMetadata(name: String) {
+final case class ZipArchiveMetadata(name: String)(val size: Long, val lastModification: Instant) {
Review Comment:
one param list please - not 2 - to match TarArchiveMetadata
--
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: notifications-unsubscribe@pekko.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@pekko.apache.org
For additional commands, e-mail: notifications-help@pekko.apache.org
Re: [PR] FILE: Add size and date information to ZipArchiveMetadata [pekko-connectors]
Posted by "jypma (via GitHub)" <gi...@apache.org>.
jypma commented on code in PR #1002:
URL: https://github.com/apache/pekko-connectors/pull/1002#discussion_r1988723894
##########
file/src/main/scala/org/apache/pekko/stream/connectors/file/model.scala:
##########
@@ -25,11 +25,13 @@ object ArchiveMetadata {
def create(filePath: String): ArchiveMetadata = new ArchiveMetadata(filePath)
}
-final case class ZipArchiveMetadata(name: String) {
+final case class ZipArchiveMetadata(name: String, size: Long, lastModification: Instant) {
Review Comment:
Should we tag those `deprecated`, since we'd have to set `size` and `lastModification` to zero? I thought about that for the old `create()` method as well.
--
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: notifications-unsubscribe@pekko.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@pekko.apache.org
For additional commands, e-mail: notifications-help@pekko.apache.org
Re: [PR] FILE: Add size and date information to ZipArchiveMetadata [pekko-connectors]
Posted by "pjfanning (via GitHub)" <gi...@apache.org>.
pjfanning commented on PR #1002:
URL: https://github.com/apache/pekko-connectors/pull/1002#issuecomment-2713297241
Could you add a test case that checks the new metadata fields. Possibly something like the Tar case https://github.com/apache/pekko-connectors/blob/401cb12e93ca75790435e2dc0578387903d62d3b/file/src/test/scala/org/apache/pekko/stream/connectors/file/impl/archive/TarArchiveEntrySpec.scala#L25
--
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: notifications-unsubscribe@pekko.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@pekko.apache.org
For additional commands, e-mail: notifications-help@pekko.apache.org
Re: [PR] FILE: Add size and date information to ZipArchiveMetadata [pekko-connectors]
Posted by "pjfanning (via GitHub)" <gi...@apache.org>.
pjfanning commented on code in PR #1002:
URL: https://github.com/apache/pekko-connectors/pull/1002#discussion_r1988799328
##########
file/src/main/scala/org/apache/pekko/stream/connectors/file/model.scala:
##########
@@ -25,11 +25,16 @@ object ArchiveMetadata {
def create(filePath: String): ArchiveMetadata = new ArchiveMetadata(filePath)
}
-final case class ZipArchiveMetadata(name: String) {
+final case class ZipArchiveMetadata(name: String)(val size: Long, val lastModification: Instant) {
def getName() = name
}
object ZipArchiveMetadata {
- def create(name: String): ZipArchiveMetadata = ZipArchiveMetadata(name)
+ @deprecated("Use the variant with 3 arguments instead.", since = "pekko-connectors-file:1.1.0")
Review Comment:
1.1.0 is already released - so 1.1.1 should be here and don't prefix it with `pekko-connectors-file:`
--
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: notifications-unsubscribe@pekko.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@pekko.apache.org
For additional commands, e-mail: notifications-help@pekko.apache.org
Re: [PR] FILE: Add size and date information to ZipArchiveMetadata [pekko-connectors]
Posted by "jypma (via GitHub)" <gi...@apache.org>.
jypma closed pull request #1002: FILE: Add size and date information to ZipArchiveMetadata
URL: https://github.com/apache/pekko-connectors/pull/1002
--
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: notifications-unsubscribe@pekko.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@pekko.apache.org
For additional commands, e-mail: notifications-help@pekko.apache.org
Re: [PR] FILE: Add size and date information to ZipArchiveMetadata [pekko-connectors]
Posted by "raboof (via GitHub)" <gi...@apache.org>.
raboof commented on code in PR #1002:
URL: https://github.com/apache/pekko-connectors/pull/1002#discussion_r1988758256
##########
file/src/main/scala/org/apache/pekko/stream/connectors/file/model.scala:
##########
@@ -25,11 +25,13 @@ object ArchiveMetadata {
def create(filePath: String): ArchiveMetadata = new ArchiveMetadata(filePath)
}
-final case class ZipArchiveMetadata(name: String) {
+final case class ZipArchiveMetadata(name: String, size: Long, lastModification: Instant) {
Review Comment:
I don't have a strong preference, so up to you as far as I'm concerned!
--
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: notifications-unsubscribe@pekko.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@pekko.apache.org
For additional commands, e-mail: notifications-help@pekko.apache.org
Re: [PR] FILE: Add size and date information to ZipArchiveMetadata [pekko-connectors]
Posted by "raboof (via GitHub)" <gi...@apache.org>.
raboof commented on code in PR #1002:
URL: https://github.com/apache/pekko-connectors/pull/1002#discussion_r1988720154
##########
file/src/main/scala/org/apache/pekko/stream/connectors/file/model.scala:
##########
@@ -25,11 +25,13 @@ object ArchiveMetadata {
def create(filePath: String): ArchiveMetadata = new ArchiveMetadata(filePath)
}
-final case class ZipArchiveMetadata(name: String) {
+final case class ZipArchiveMetadata(name: String, size: Long, lastModification: Instant) {
Review Comment:
this seems like it'd not be binary compatible, because of the generated methods for case classes that would change - I'm not sure if it's possible to manually 'add back' the original methods to restore binary compatibility?
--
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: notifications-unsubscribe@pekko.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@pekko.apache.org
For additional commands, e-mail: notifications-help@pekko.apache.org
Re: [PR] FILE: Add size and date information to ZipArchiveMetadata [pekko-connectors]
Posted by "jypma (via GitHub)" <gi...@apache.org>.
jypma commented on code in PR #1002:
URL: https://github.com/apache/pekko-connectors/pull/1002#discussion_r1988721612
##########
file/src/main/scala/org/apache/pekko/stream/connectors/file/model.scala:
##########
@@ -25,11 +25,13 @@ object ArchiveMetadata {
def create(filePath: String): ArchiveMetadata = new ArchiveMetadata(filePath)
}
-final case class ZipArchiveMetadata(name: String) {
+final case class ZipArchiveMetadata(name: String, size: Long, lastModification: Instant) {
Review Comment:
Ah, I missed that. Sure, I'll try to toss in some additional `apply` methods.
--
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: notifications-unsubscribe@pekko.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@pekko.apache.org
For additional commands, e-mail: notifications-help@pekko.apache.org
Re: [PR] FILE: Add size and date information to ZipArchiveMetadata [pekko-connectors]
Posted by "jypma (via GitHub)" <gi...@apache.org>.
jypma commented on PR #1002:
URL: https://github.com/apache/pekko-connectors/pull/1002#issuecomment-2724010709
Unfortunately, the size addition in this PR turned out to be a red herring. `getSize()` on `ZipInputStream` apparently happily returns -1, even though the size is known in the Zip TOC. I'm seeing what's also reported [here](https://stackoverflow.com/questions/36045421/java-zipentry-getsize-returns-1).
Additionally, it seems that unfortunately currently there are no tests for the "unzipping" part that's modified here. `ZipArchiveFlowTest` seems to test the creation of ZIP files, where we wouldn't even want the user to report a size.
I'm closing this PR for now, as I've had to rewrite my code in terms of `ZipFile` anyway (instead of `ZipInputStream`), which does report size correctly. If I find the time at some point, I could help out with a re-implementation around `ZipFile` (which also seems to be more friendly in arbitrarily seeking in a ZIP file).
--
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: notifications-unsubscribe@pekko.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@pekko.apache.org
For additional commands, e-mail: notifications-help@pekko.apache.org