-
Notifications
You must be signed in to change notification settings - Fork 34
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
RFC: generalize resource interface #24
Closed
Closed
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
the proposal itself is not yet fleshed out at all - still in the information collecting phase so that the motivations are clear Signed-off-by: Alex Suraci <[email protected]>
Signed-off-by: Alex Suraci <[email protected]>
vito
changed the title
RFC: generalized resource interface
RFC: generalize resource interface
Apr 3, 2019
Signed-off-by: Alex Suraci <[email protected]>
...plus a bunch of tweaks to make the markdown linter happy Signed-off-by: Alex Suraci <[email protected]>
Signed-off-by: Alex Suraci <[email protected]>
Signed-off-by: Alex Suraci <[email protected]>
Signed-off-by: Alex Suraci <[email protected]>
Signed-off-by: Alex Suraci <[email protected]>
I decided to stray from the initial v2 doc, which had a 'reset' event type and a 'discovered' event type. Instead 'check' only returns a stream of JSON objects containing config and metadata. Signed-off-by: Alex Suraci <[email protected]>
Since `put` can both create and delete configs, it seems better to keep them all using the same event structure, even if 'check' can only emit one type of event (`discovered`). The alternative to this would be to split deleting out of 'put'. At that point they would each emit a single type of event, and the interface would be less open-ended. There's value in that, but that value has to be weighed against the value of being able to create + delete in one action. It's also not super clear that all resource types would support 'delete'. (But then, not all of them will support 'put', either.) Signed-off-by: Alex Suraci <[email protected]>
Signed-off-by: Alex Suraci <[email protected]>
this removes 'event types' from the response from each action, and now they all have the same response. event types were needed because 'put' could both create and delete, and we needed a way to tell the difference. so instead, we've split 'delete' out, and now each action has only one way to interpret the events, so the types are no longer needed. this makes the interface a bit less flexible, but it seems worth seeing how far we can go with this constraint for the sake of having a simpler, more focused interface. if we can go the distance with this limitation it feels like stronger validation of the resources concept. along the way I've put the configuration for each command under an 'actions' field in the info response. i'd like to also have it so actions are optional, i.e. you can have a resource that doesn't implement 'delete' or 'put'. this seems like it'll be necessary if we want a common resource interface to be used for things like notifications or triggers where there's nothing to really delete, and even today there are cases where resources are 'read-only' and don't support put at all (e.g. RSS). Signed-off-by: Alex Suraci <[email protected]>
we were already relying on image semantics here by expecting it to be in the working directory, so relying on CMD should be no worse. this will allow greater flexibility for resource type authors, allowing for a single-binary entrypoint rather than forcing them to have an 'info' executable in a particular location. Signed-off-by: Alex Suraci <[email protected]>
Signed-off-by: Alex Suraci <[email protected]>
check no longer emits 'reset', it emits the given version first to show it still exists (just as today) Signed-off-by: Alex Suraci <[email protected]>
Signed-off-by: Alex Suraci <[email protected]>
these all go hand-in-hand so they have the same RFC number and are in the same PR, but they each deserve their own proposals Signed-off-by: Alex Suraci <[email protected]>
vito
force-pushed
the
generalized-resources
branch
from
April 8, 2019 17:00
ca06f53
to
80f3ce1
Compare
Signed-off-by: Alex Suraci <[email protected]>
this will allow us to have a common mechanism for configuring TLS across all resource types. having it be a formal part of the interface feels sensible as all resources are meant to interact with external state - and in doing so, TLS will be a commonplace. a later RFC (or perhaps another proposal within this one) will demonstrate how users configure TLS - it may be something like the pipeline-level, or another field alongside 'source' on the resource definitions. Signed-off-by: Alex Suraci <[email protected]>
this is not well-defined yet and this is easy enough to do as a later iteration of the resource interface, now that it's versioned. Signed-off-by: Alex Suraci <[email protected]>
This was referenced Apr 9, 2019
vito
commented
May 10, 2019
This was referenced May 10, 2019
Closed
vito
commented
May 13, 2019
fixes concourse/concourse#3789 Signed-off-by: Alex Suraci <[email protected]>
Signed-off-by: Alex Suraci <[email protected]>
* introduce artifacts: and how it can be used as a migration path for explicit `get` after `put` * update open/closed questions (they are all now closed) Signed-off-by: Alex Suraci <[email protected]>
This was referenced May 21, 2019
Closed
Open
each of these proposals should be submitted as separate RFCs. each RFC should show how the v2 *and* v1 interfaces are to be interpreted. i've also removed the artifact resources proposal, which I initially included in order to provide a concrete example relating today's resource usage to the v2 interface. however, it can technically be done independently of the v2 interface, if not just as a way of achieving 'explicit `get` after `put`' in a backwards-compatible way. so let's just propose it separately and link to all four to demonstrate how it is just one of four proposed interpretations. Signed-off-by: Alex Suraci <[email protected]>
these are being made independent of v1 vs. v2 Signed-off-by: Alex Suraci <[email protected]>
calling this something that sounds like better english until someone has a good reason otherwise Signed-off-by: Alex Suraci <[email protected]>
Signed-off-by: Alex Suraci <[email protected]>
vito
commented
Jun 3, 2019
* Examples: `{"uri":"https://github.com/concourse/concourse"}`, `{"interval":"10m"}` | ||
|
||
* **Config fragment**: a smaller JSON object intended to be "spliced" into a **config** by assigning each field from the fragment into the config. | ||
* Examples: `{"ref":"abcdef"}`, `{"branch":"develop"}` |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We should be careful with the allowed syntax for the keys - i.e. disallow :
since we use it for version syntax in fly check-resource
.
This was referenced Aug 13, 2019
Merged
closing in favor of #37 |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Rendered
Status: stable draft
Please leave comments on individual lines so that conversations can be threaded and resolved. Comments left at the toplevel unnecessarily will be
brutally destroyedmaintained so that the conversation doesn't get out of hand.Related RFCs which specify how this interface is interpreted:
across
step #29 (spatial resources)Previous discussions: