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

Stored ImageStreams are invalid #92

Closed
blschatz opened this issue May 31, 2019 · 4 comments
Closed

Stored ImageStreams are invalid #92

blschatz opened this issue May 31, 2019 · 4 comments

Comments

@blschatz
Copy link
Contributor

When not using compression, libaff4 generates the wrong metadata for the relevant ImageStream. Eg:

aff4:compressionMethod aff4:NullCompressor ;

In this case it should not store a compressionMethod. From the spec v1

"Where compression is used within the Image Stream, the object must have a property aff4:compressionMethod set to a resource identifying the compression algorithm. Where there is no compressionMethod set, it is assumed that chunks are stored."

@blschatz blschatz changed the title Stored images are invalid Stored ImageStreams are invalid May 31, 2019
@scudette
Copy link
Collaborator

scudette commented May 31, 2019

Reading that paragraph does not seem to suggest that this is incorrect behavior:

  1. If we use compression we have to set the property
  2. If the property is not set then we assume the null compressor (i.e. the default setting is null compressor).

We are setting the null compressor explicitly which is not forbidden. It is also clearer since we are less open to interpretations of the default value.

scudette added a commit that referenced this issue May 31, 2019
scudette added a commit that referenced this issue May 31, 2019
@scudette
Copy link
Collaborator

Ahh sorry - this commit actually fixes issue #94

@scudette
Copy link
Collaborator

So the main issue here is that aff4:NullCompressor is actually not defined in the spec at all. Instead the spec suggests that if the property is omitted it indicates that null compression is implied: i.e. if the writer intends to indicate no compression was used they should not include the property at all.

We have a couple of options:

  1. Change the code to never emit a property if there is no compression at all. This is similar to the evimetry behavior. Evimetry currently chokes on the aff4:NullCompressor as an unknown compressor and the only way to imply no compression is to not have the property at all.

  2. Extend the Evimetry tool to understand aff4:NullCompressor as an extension to the spec, while at the same time introducing the aff4:NullCompressor in the 1.1 spec revision. I feel it is better to have a way to properly specify no compression rather than relying on an absence of specification and an implied default value.

  3. For c-aff4 we will continue recognizing the aff4:NullCompressor anyway so we can continue reading images which we produced in the past and specify it. While if the property is not specified at all we set aff4:NullCompressor as the default - this way we can continue reading Evimetry images too.

In a sense the issue is that Evimetry is more strict in the type of images it is willing to read so we need to produce images that follow the exact interpretation of the spec to make sure our images are properly supported by Evimetry readers. We should continue to be flexible in the type of images we can read though and accept either way if we can.

I feel that the path of least resistance is for us to change to increase interoperability - moving forward Evimentry can read our stored images which it can not now and we can read our stored images as normal either way (old ones or new ones). The discussion about the 1.1 standard is orthogonal and separate.

@blschatz
Copy link
Contributor Author

blschatz commented Jun 4, 2019

it turns out that we have been using the NullCompressor lexicon element for this in Evimetry all along - it is only missing from pyaff4. Given this is the case, I no longer think we need to take the spec so literally, and think it is safe to treat the NullCompressor as errata to the 1.0 spec and fix pyaff4.

@blschatz blschatz closed this as completed Jun 4, 2019
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

2 participants