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

Say `Expect: 202-digest` intended for use with `HEAD`? #32

Closed
zimeon opened this issue Feb 1, 2017 · 36 comments · Fixed by #84

Comments

@zimeon
Copy link
Contributor

commented Feb 1, 2017

Related to #31, would it make sense to add to Section 3.3 Persistence Fixity preamble a statement that the Expect: 202-digest is intended to be used with HEAD? Also may be used with GET (though kinda pointless since one gets the bits in that case)?

@ajs6f

This comment has been minimized.

Copy link
Contributor

commented Feb 2, 2017

I'm not sure that it's true that use with GET is pointless. The idea may be "I want the bits but as I get them let's make sure they're the right ones." Fixity can be understood as an explicit act or as a quality of retrieval. (And lots of other ways, too.)

@ajs6f

This comment has been minimized.

Copy link
Contributor

commented Feb 2, 2017

I don't how anyone could expect end-to-end message integrity from Fedora: if you put two committers in a room and ask them a question you get three opinions! Oh, I kill me...

@barmintor

This comment has been minimized.

Copy link
Contributor

commented Feb 2, 2017

FWIW: I wrote to GET with the understanding that HEAD is identical except for the presence of a response entity, per HTTP. I would typically use HEAD for this, too.

@barmintor

This comment has been minimized.

Copy link
Contributor

commented Feb 2, 2017

@ajs6f

This comment has been minimized.

Copy link
Contributor

commented Feb 2, 2017

I think this issue is half-on half-off. An explanatory note pointed downwards to say "This is how you would use this." is a good idea. Singling out HEAD is not.

@zimeon

This comment has been minimized.

Copy link
Contributor Author

commented Feb 3, 2017

I agree that HEAD = headers of GET response, also fine with the idea that the fixity check can be used with GET although that isn't the sense in which the current description is written.

Up to section 3.3 Persistence Fixity the document is very clear about applicable HTTP methods so I think it should be explicit here to. My understanding now is that 202-digest MUST (based on comments in #33) be supported for HEAD and GET (one could imagine use as a pre-condition for PUT/PATCH/DELETE but I don't think that is/was intention). To make this clear we could change:

In the minimal case, a client that has an original digest value can verify persistence fixity by downloading the LDP-NR and calculating the checksum. To avoid the necessity of transferring a potentially large binary, this specification describes a new expectation token for the Expect header described in [RFC7231]: 202-digest, and a digest-link parameter.

to

In the minimal case, a client that has an original digest value can verify persistence fixity by downloading the LDP-NR and calculating the checksum. To avoid the necessity of transferring a potentially large binary, this specification describes a new expectation token for the Expect header described in [RFC7231]: 202-digest, and a digest-link parameter. An implementation MUST support the 202-digest expectation for HEAD and GET requests.

@awoods

This comment has been minimized.

Copy link
Collaborator

commented Feb 9, 2017

@acoburn / @ajs6f / @barmintor : are you in agreement with @zimeon's suggested update to the text?

@barmintor

This comment has been minimized.

Copy link
Contributor

commented Feb 9, 2017

I don't think so:

  1. The MUST is in the wrong place (it should be in 1.5.2)
  2. I don't think the MUST is appropriate
  3. We could more effectively address this by adding a section 1.6 for HEAD and say non-normatively that HEAD is expected to be identical to GET, but without a response entity, following LDP and HTTP more generally.
@ajs6f

This comment has been minimized.

Copy link
Contributor

commented Feb 9, 2017

I think we're talking about the same content (modulo @barmintor's point about the location of MUST). I'm fine with any disposition of the content that seems most clear to the most people. I like having a MUST for HEAD. I would prefer it.

@zimeon

This comment has been minimized.

Copy link
Contributor Author

commented Feb 9, 2017

I feel that in this issue and #33 we are going round in circles because we haven't agreed whether fixity support is mandatory in a conforming Fedora API Specification implementation --- I can see a consistent specification that goes either way: 1) implementations MUST implement section 3 or 2) implementations MAY implement section 3 and here is how you can tell. IMO, the specification should be crystal clear on which case applies. (Perhaps the same question applies to other sections also 2, 4, 5, 6)

@ajs6f

This comment has been minimized.

Copy link
Contributor

commented Feb 9, 2017

+1 to crystal clarity.

@awoods

This comment has been minimized.

Copy link
Collaborator

commented Feb 9, 2017

I believe the primary point of the Fedora Specification is to define the core behaviors that are expected of a Fedora Repository. Therefore, I would suggest that we make it clear that all sections of the spec (1-6) are required... noting, of course, that each section offers inherent flexibility.

@barmintor : would you be strongly opposed to language along the lines of: An implementation MUST support the 202-digest expectation?

@ajs6f

This comment has been minimized.

Copy link
Contributor

commented Feb 9, 2017

+1 to that (requiring the behavior).

@zimeon

This comment has been minimized.

Copy link
Contributor Author

commented Feb 9, 2017

+1 from me also on requiring support for all sections of the spec. I wasn't part of the original discussions but that seems to give sense to what "A Fedora Repository" means, rather than just a set of features that Fedora-like repositories may or may not implement.

@ajs6f

This comment has been minimized.

Copy link
Contributor

commented Feb 9, 2017

I'll say again that I'm sympathetic to @barmintor's point about having some common understanding of what impls SHOULD do when they can't or won't do the right thing. But I think this very discussion has proved that such guidance can also actually create confusion as much as resolve it.

@barmintor

This comment has been minimized.

Copy link
Contributor

commented Feb 9, 2017

@awoods here is my feeling about this broadly: if somebody says "can I run Hydra against X?", and there's a detectable gap in conformance that can be worked around- "You got a status that suggests the server didn't verify, so do it yourself"- but things otherwise work, then I don't know what the point of MUST was. If I can't know that it doesn't conform, or departure will prompt misinformed/misleading client interactions, then the MUST matches the situation. If backfilling is cumbersome, that seems like a SHOULD to me. I am mostly interested in clarifying things for the client/integrating systems. I think it is unfortunate if we use MUST in a way that doesn't reflect those circumstances, but I also don't think it's worth worrying about.

I am approaching the spec from the perspective that your suggestion about conformance of all sections simply will not be true, or at least relevant. If an implementation doesn't support messaging... well, who cares? If it claims to support the messaging spec, then I care what that means. If it doesn't, but it supports CRUD, and that's what I need, then that's fine. If the idea is that the spec embodies the minimal set of common features to support a repository and we're going to think the "100% Fedora Approved!" imprimatur is meaningful, then I honestly think the spec should only include section 1. 2-6 should be separate specs.

Because of this, I am more interested in well-defined, testable behaviors that can be verified in a test suite. The LDP conformance report is littered with red text. I don't know why ours wouldn't be. The text of the spec doesn't have to support that, but I'm pretty sure practice will. The client platforms will have to document the specs they require to identify suitable backends.

@ajs6f

This comment has been minimized.

Copy link
Contributor

commented Feb 9, 2017

If an implementation doesn't support messaging... well, who cares?

I find this remark bizarre. If this is our attitude, then WTF is the purpose of this effort at all? We are publishing vague suggestions about what we would like people to do if they want to label their work as Fedora?

@ajs6f

This comment has been minimized.

Copy link
Contributor

commented Feb 9, 2017

The client platforms will have to document the specs they require to identify suitable backends.

I think this is stunningly unrealistic because it imagines a world in which there are far more client platforms than I believe there ever will be. Fedora is just not important enough for it to assume that it forms a center of gravity for any community other than the one it has grown for itself.

@barmintor

This comment has been minimized.

Copy link
Contributor

commented Feb 9, 2017

I think these specifications are useful in context, but I also think context is not universal. The time I've spent on them is evidence that I think the effort is worthwhile. The principal context in which I use Fedora arguably already has 2 back ends, not including Cavendish. I'm invested in specifications that help that client context negotiate different backends.

@ajs6f

This comment has been minimized.

Copy link
Contributor

commented Feb 9, 2017

I've made my opinions plain, but maybe I badly misunderstand what is going to be useful.

I invite @awoods to use his voice to gather some sense of what the larger community will find useful in this circumstance. A series of isolated specifications, to which implementations pledge their allegiance on an a-la-carte basis, seems thoroughly pointless to me, and if that's where this ends up going, I will certainly stop working on it and instead spend my time working in the communities from which we drew for this work (Memento, Solid, etc.).

@zimeon

This comment has been minimized.

Copy link
Contributor Author

commented Feb 9, 2017

+1 to getting more community input (put on email list?). To me this question is fundamental to how to describe the features in a way that makes the specification understandable/usable/meaningful. Someone implementing needs to know "what behavior must I implement?" and someone building a client needs to know "what behavior can I expect?".

An overall notion of "is a Fedora Repository" is a big out-of-band signal about lots of behaviors. A feature-by-feature approach likely needs discovery and/or fallbacks documented. Of course one could do something in the middle and says sections/specs a,b,d must be implemented, c,e,f are optional (and support is declared/discovered via...). One way to think about this choice would seem to be a set of core use cases -- if there is community agreement on use cases for a MVP, then that would be a reasonable way to define the must-support features.

@barmintor

This comment has been minimized.

Copy link
Contributor

commented Feb 9, 2017

I think that would be a loss, particularly since the memento spec is one of the models for my clearly unsuccessful attempts at describing flexibly implemented specs.

In any case, the only remotely strong stand I could make is that if something is a MUST in a normative section, it must be able to be tested. 202-digest is not, at least not yet. There's no minimum digest algorithm specified, and there's no testable behavior around the link param except that the status was a 202.

@ajs6f

This comment has been minimized.

Copy link
Contributor

commented Feb 9, 2017

Well, we've had that disagreement before as well, @barmintor. There is plenty of untested material already here, and we're going to add more. I think your requirement is unrealistic.

@zimeon

This comment has been minimized.

Copy link
Contributor Author

commented Feb 9, 2017

This is obviously a thread going out in lots of directions... but if testability is a key issue then could there be agreement on a minimal set of digests (perhaps one) that MUST be supported?

@mjgiarlo

This comment has been minimized.

Copy link

commented Feb 15, 2017

This is a long thread and I'm coming in late...

There was some agreement expressed throughout this discussion, and much of the current disagreement seems rooted in the question, "must an implementation support all six sections of the spec to be considered Fedora?" Reading over the latest draft of the spec, I wonder if a middle-road approach might be worth considering here as a compromise (and if I've totally missed the point of the disagreement, I offer my apologies): how about if, say, sections 1-4 are required and sections 5-6 are optional? Think of them as tiers of compliance with (completely arbitrary example here) tier 1 being Fedora implementations including sections 1-4; tier 2 adding section 6; and tier 3 adding section 5 (if, judging by @acoburn's earlier remark, atomic operations are more problematic in some implementations).

@zimeon

This comment has been minimized.

Copy link
Contributor Author

commented Feb 16, 2017

I like @mjgiarlo's suggestion of a core that must be implemented to be Fedora (perhaps with optional additional levels of compliance for optional features). Being explicit about this would be a big help.

However, the more I think about what compliance means ... I tend toward testable in the sense that @barmintor mentioned in #32 (comment) -- and that requires things like mandating at least one digest algorithm (or, even weaker, at least one from a specified set). Perhaps having a more expansive set of requirements that are tested to give a matrix like the LDP compliance report -- even with some FAIL entries -- might actually be rather more useful for assessing repository fitness for a particular purpose than a set of hollow "follows Fedora spec" claims?

@mjgiarlo

This comment has been minimized.

Copy link

commented Feb 16, 2017

I'll ask a hopefully simple question: is what we're discussing now still the issue written above? Or are we off in the wilds of Tangentistan? It's not clear to me that two weeks of discussion has gotten this issue closer to done, though it's surfaced a healthy back-and-forth.

@mjgiarlo

This comment has been minimized.

Copy link

commented Feb 16, 2017

@acoburn OK. Yes, my examples were totally (well, mostly totally 😉) arbitrary, in an attempt to test a compromise approach. Would that it could be so simple...

@ajs6f

This comment has been minimized.

Copy link
Contributor

commented Feb 16, 2017

@mjgiarlo It's good of you to try, but the fundamental disagreement is not about the content of the document, but its purpose. "Splitting the difference" won't work to resolve that.

For myself, @acoburn is right. "Optional" chapters, or services, or features don't work. This document was explicitly and very publicly advertised as the minimal definition of Fedora. If there are optional portions in it, it is not minimal, and if it is minimal, you cannot remove anything from it without destroying it.

@mjgiarlo

This comment has been minimized.

Copy link

commented Feb 16, 2017

Does adding An implementation MUST support the 202-digest expectation for HEAD and GET requests., as suggested by @zimeon above, materially affect our ability to test implementations against the spec (what seems to be stuck in @barmintor's craw)?

Pardon if I'm darting through the trees whilst y'all survey the forest. ;)

@tpendragon

This comment has been minimized.

Copy link

commented Feb 17, 2017

I'm largely of the opinion here that I need to be able to write a useful generic Fedora client, and @barmintor's points strike home in that regard. As written the spec gives only enough information for 202-digest that I could send along the request as given to me (the fedora client) and give back the response. I can provide no guidance as to what inputs are valid for digest-param and digest-link-param, but I CAN tell them either "whatever it was you gave me wasn't supported (I got back 417) or it WAS supported and it failed to match (412)." That seems enough to me. Anyone who uses the client would either see 417 (and MUST here for an implementation that does not support any kind of fixity I assume would just send 417 for everything with that header) and try another digest if they have one, or move on and do it the expensive way (download/recalculate).

All of this to say...👍 for 202-digest being required, 👎 to requiring any particular digest algorithm (as long as if the answer is none it just always returns 417), 👍 to not using 412/417 for anything else that could be in the headers.

Regarding testability...I'm for SHOULD for md5. It's definitely lowest impact, and you should be able to test a made-up algorithm for 417 support.

Edit: Also, digest-link-param is 100% untestable, I think. It's basically "here's a spot to put any old URI and the server may or may not figure out what to do with it." That's...probably fine? Better to offer up a spot than to deny that use case outright, unless that use case doesn't really exist.

@ajs6f

This comment has been minimized.

Copy link
Contributor

commented Feb 20, 2017

That use case most certainly does exist. I have A/V archivists asking me for it now.

@anarchivist

This comment has been minimized.

Copy link

commented Feb 22, 2017

To bring this back full circle, @barmintor's response regarding @zimeon's feedback:

  1. The MUST is in the wrong place (it should be in 1.5.2)
  2. I don't think the MUST is appropriate
  3. We could more effectively address this by adding a section 1.6 for HEAD and say non-normatively that HEAD is expected to be identical to GET, but without a response entity, following [LDP] and HTTP more generally.

I'm definitely 👍 on 1 (MUST is in the wrong place) and 3 (add §1.6 for HEAD).

As for 2, the lack of guidance and testability identified by @tpendragon builds on @barmintor's concerns. MUST does not feel appropriate with the behavior as written; the behavior is not testable. We could, arguably, break the behavior down further to reflect our ability to review conformance.

I propose we draw a line in this issue about the overall structure and purpose of the specs - this is a big issue, and perhaps benefits from further discussion as a distinct topic.

@ruebot

This comment has been minimized.

Copy link
Contributor

commented Apr 5, 2017

I propose we draw a line in this issue about the overall structure and purpose of the specs - this is a big issue, and perhaps benefits from further discussion as a distinct topic.

So we don't lose track of this one, would it be worth coordinating with folks on this ticket on a Fedora Tech call we could be at and discuss further?

@zimeon

This comment has been minimized.

Copy link
Contributor Author

commented Apr 6, 2017

Getting back to the original issue here -- indication of what HTTP methods the 202-digest expectation applies to -- there seems to be agreement in the thread above that the persistence fixity section applies to HEAD and GET, there is not agreement on my suggested use of MUST. I think the various other issues raised along the way are best handled elsewhere so I propose that we add a sentence to the start of the non-normative Persistence Fixity section that parallels the start of the Transmission Fixity section which highlights POST and PUT:

Persistence fixity is verified by comparison of stored fixity results with newly computed fixity results using HEAD or GET requests to LDP-NRs. ...

I'm going to be so bold as to make a PR for this in the hope of resolution.

zimeon added a commit to zimeon/fcrepo-specification that referenced this issue Apr 6, 2017

@ruebot ruebot closed this in #84 Apr 7, 2017

ruebot added a commit that referenced this issue Apr 7, 2017

@ruebot ruebot reopened this Apr 7, 2017

@ruebot

This comment has been minimized.

Copy link
Contributor

commented Apr 7, 2017

Addressed with: 2ea9ed2

@ruebot ruebot closed this Apr 7, 2017

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
8 participants
You can’t perform that action at this time.