Skip to content
This repository has been archived by the owner on May 9, 2020. It is now read-only.

Bug 1397373 - superseding docs are now in the queue #314

Merged
merged 1 commit into from Sep 8, 2017

Conversation

djmitche
Copy link
Contributor

@djmitche djmitche commented Sep 6, 2017

No description provided.

@djmitche djmitche self-assigned this Sep 6, 2017
@petemoore
Copy link
Member

Let's chat about this. I understand why you've done this, but I'm a little uncomfortable with it, as it is only by convention that two of our workers implement this way, and it seems like we lose something by hiding the fact that superseding can be implemented however the worker wishes.

I would say, if we want to enforce this is how superseding is implemented, we should move the supersededUrl out of the task payload into the root of the task definition, and update the worker-queue interaction docs to explain the responsibility of the worker to implement the feature. Although it has caused some pain that workers have implemented some things differently, it is great that we have a means for different workers to try out different types of features, and we're able to roll them out or change them independently. So for me, I'd still prefer to keep the supersederUrl in the task payload.

I can imagine a worker that doesn't refer to an external service to determine if a task is superseded or not (e.g. a worker running as a service, like buildbot bridge).

@djmitche
Copy link
Contributor Author

djmitche commented Sep 7, 2017

Where would you suggest the documentation go?

@petemoore
Copy link
Member

I'm not 100% sure, but I think I'd still like it in the worker docs, and the queue docs just to say what the task resolution reason "superseded" means semantically, but refer the user to the specific worker docs for implementation details. We could highlight in generic-worker and docker-worker docs that the implementation is the same as the other, if we want to save the user from needing to discover that after reading both.

@djmitche
Copy link
Contributor Author

djmitche commented Sep 7, 2017

Could this be a recommendation from the queue? I'd like for taskcluster-worker, or whatever follows it, to feel encouraged to do something similar!

@djmitche
Copy link
Contributor Author

djmitche commented Sep 7, 2017

I updated this to be a recommendation in the queue.

Copy link
Member

@petemoore petemoore left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks Dustin!

@djmitche
Copy link
Contributor Author

djmitche commented Sep 8, 2017

Thanks for your help :)

@djmitche djmitche merged commit d289448 into taskcluster:master Sep 8, 2017
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants