-
Notifications
You must be signed in to change notification settings - Fork 18
Create an artifact plugin #55
Conversation
|
@jonasfj, @gregarndt: How does this payload schema look to you? Let's change it to be what we want. |
plugins/artifacts/config-schema.yml
Outdated
| title: Local artifact location | ||
| description: Filesystem path of artifact | ||
| type: string | ||
| remotePath: |
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.
I propose path and name...
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.
remotePath/name should not start with /...
If type is folder it should end with /, and if type is file it should not ever end with /...
Also it should never contain any double slashes...
|
Looks good, we just need to get a consensus on the naming of the source and destination stuff. |
|
I propose Honestly, I can't care much... As long as we don't call it Regarding |
|
I submit a further WIP diff to check and see if this is a good direction to be going in. I feel fairly happy with how this is shaping up. If this is a generally acceptable way of going about this, most of the changes from here on out will be filling out error handling in the plugin itself, and using the client lib stuff (#60) that you all are working on now in the runtime bits. Currently the tests for this look like |
|
A note for @gregarndt: We decided to move ReadSeekCloser to runtime to avoid dependency loops between runtime and engine. |
|
I'd love if I could get a 30% review on this at some point today, just so that we all feel like this is moving in the right direction. As a caveat, nothing is done and the points don't matter. If this might be better done as a guided excercise at this point, I'd be happy to hop on vidyo and talk it through with someone. @jonasfj, @gregarndt: any takers? |
|
|
||
| func (plugin) NewTaskPlugin(options plugins.TaskPluginOptions) (plugins.TaskPlugin, error) { | ||
| return &taskPlugin{ | ||
| TaskPluginBase: plugins.TaskPluginBase{}, |
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.
No need for this line... the zero-value is assigned by default...
|
@gregarndt, @jonasfj: Alright, I think this is pretty much ready (barring inevitable review comments). In particular, I'm pretty sure the error handling I have in here is still overly simplistic, but it's at least moving in the right direction I think. Example of created artifacts: https://tools.taskcluster.net/task-inspector/#WzLXbfiVTf2MHmaCEJFRmg/0 |
runtime/ioext/ioext.go
Outdated
| } | ||
|
|
||
| // ReadSeekNopCloser is an implementation of ReadSeekCloser that wraps io.ReadSeekers | ||
| type ReadSeekNopCloser struct { |
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.
You don't need to export this type...
You create it with NopCloser whose return type should ReadSeekCloser
Does what it says on the tin.