Split claim and claim result into separate documents

carolynvs commented 4 years ago

This is follow-up to #339

Split the claim document into two documents: Claim and Claim Result. A claim may have multiple results if it is attempted multiple times (for example it is cancelled and retried).
I noticed that result message was marked as required in the schema but optional in the spec document, and was optional in cnab-go. I added a commit that changes the result message to optional. If it was intended to be required, I can flip that commit around to update the spec text instead.
When claim and result were on the same document, we briefly considered splitting created and modified between the top level claim and the result. Undo that change and have a created/modified pair for both claim and result. Tools may choose to treat claim as immutable, but that isn't defined in the spec.
Mark both created/modified as required, matching the schema and how they are used in cnab-go.
Allow revision to be updated for non-modifying actions.

Closes #328

technosophos commented 4 years ago

Sorry about all the technosophos references. I did a purge on all of those, but must have accidentally re-introduced them on a subsequent merge. I will fix those in a follow-up PR. Don't worry about it.

carolynvs commented 4 years ago

I have pushed a new commit that changes revision so that implementations MAY update it for non-modifying actions. This should address https://github.com/cnabio/cnab-spec/pull/349#discussion_r391054936.

From the commit message:

To support results referencing back to claims for non-modifying actions using only the installation name + revision, allow an implementation to assign a new revision for non-modifying actions.

Implementations that intend to persist results for every action will use this to assign a new revision to the claim for every action (modifying or otherwise) to guarantee that that the reference is unique. Implementations that chose to only persist the last modifying action or only persist the revisions of modifying actions, can continue assigning the revision as they do today.

This seems like a smaller change while allowing the implementations room to implement various aspects as they see fit (immutable, indexing by content digest, overwriting the claim file, keeping just the revisions of modifying actions like update but not stuff like logs, etc).

carolynvs commented 4 years ago

Rebased to pick up the latest changes to the CI checks.

technosophos commented 4 years ago

I believe this just needs a force-push sign-off from @carolynvs and we are ready to merge this, right? (Or are there still unaddressed issues?)

carolynvs commented 4 years ago

I'd be up for adding an id attribute to claims

Great! I know that wasn't palatable before. Let me rework the PR today to address your feedback.

carolynvs commented 4 years ago

@technosophos @jlegrone I reverted my last commit since it caused unanticipated problems with DataDog's implementation.

Please take a close look as I am altering or perhaps clarifying terms and fields that different implementations have had different interpretations for. 😀

Installation refers to the installation of the bundle
Claim records the execution of an action against an installation
Claims have an id that results reference
An installation has many claims with these new definitions.
Claims and results are immutable. Each new action against an installation generates a new claim. Each new status transition of an operation could yield a new result (depending on the implementation).
Clarify that the last modifying claim is retained (same as before) unless the implemenation wants more history. The last result is retained for a retained claim (again unless an implemenation wants more history).

Depending on your previous interpretation of the spec, this may change what claim.created means to you. The meaning has hopefully been clarified to the created date of the claim (see updated definition above), not the installation.

Both Porter and Duffle would update to keep more history. A more simple implementation could choose to keep a single file around per installation with the last claim revision and the associated result. It would lose the built in answer to "When was the entire installation first created?" (see above) but could put that into claim.custom if it cared to know, or could keep around the first revision, and along with the last.

jlegrone commented 4 years ago

I think this closes https://github.com/cnabio/cnab-spec/issues/328?

carolynvs commented 4 years ago

This is gonna be an epic squash

womens squash tournament

cnabio / cnab-spec

Split claim and claim result into separate documents #349