Skip to content

feat(hooks): add ability to specify 'taskId' when triggering hooks#8287

Merged
matt-boris merged 1 commit intotaskcluster:mainfrom
ahal:ahal/push-xuxzmrtlpskq
Mar 3, 2026
Merged

feat(hooks): add ability to specify 'taskId' when triggering hooks#8287
matt-boris merged 1 commit intotaskcluster:mainfrom
ahal:ahal/push-xuxzmrtlpskq

Conversation

@ahal
Copy link
Contributor

@ahal ahal commented Feb 17, 2026

No description provided.

@ahal ahal force-pushed the ahal/push-xuxzmrtlpskq branch 2 times, most recently from 34f765f to 5b9c48d Compare February 17, 2026 21:22
@ahal ahal marked this pull request as ready for review February 17, 2026 21:23
@ahal ahal requested a review from a team as a code owner February 17, 2026 21:23
@ahal ahal requested review from lotas, matt-boris and petemoore and removed request for a team February 17, 2026 21:23
@ahal ahal force-pushed the ahal/push-xuxzmrtlpskq branch from 5b9c48d to bb3c913 Compare February 17, 2026 21:37
Copy link
Contributor

@matt-boris matt-boris left a comment

Choose a reason for hiding this comment

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

Changes LGTM, thank you for implementing!

Noted the breaking client changes, just requesting the changelog be updated please 🙏🏻

@ahal ahal force-pushed the ahal/push-xuxzmrtlpskq branch from bb3c913 to cf05752 Compare March 3, 2026 15:45
@ahal ahal requested review from lotas and matt-boris March 3, 2026 15:46

try {
resp = await this.taskcreator.fire(hook, context);
resp = await this.taskcreator.fire(hook, context, options);
Copy link
Contributor Author

Choose a reason for hiding this comment

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

I realized that if I extracted the taskId here, then we wouldn't need to rely on hook authors to set it in the JSON-e template..

But if this crosses some sort of boundary, we could instead extract it from the rendered task definition.

@ahal
Copy link
Contributor Author

ahal commented Mar 3, 2026

I started having some difficulty spinning up a local instance (getting "no matching manifest for linux/amd64 in the manifest list entries" errors on yarn start). So I didn't manage to test this locally :/

@ahal ahal marked this pull request as draft March 3, 2026 15:50
@matt-boris
Copy link
Contributor

I started having some difficulty spinning up a local instance (getting "no matching manifest for linux/amd64 in the manifest list entries" errors on yarn start). So I didn't manage to test this locally :/

Which containers are you getting this for? I've run into this as well, and have DOCKER_DEFAULT_PLATFORM set to linux/amd64 locally, so sometimes I just unset this var and try yarn dev:start up again and it works.

@ahal
Copy link
Contributor Author

ahal commented Mar 3, 2026

I just filed:
#8341

Let's move discussion there

@ahal ahal force-pushed the ahal/push-xuxzmrtlpskq branch from cf05752 to f0e733c Compare March 3, 2026 16:19
@ahal ahal marked this pull request as ready for review March 3, 2026 16:31
Copy link
Contributor

@lotas lotas left a comment

Choose a reason for hiding this comment

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

LGTM! Thanks

@ahal
Copy link
Contributor Author

ahal commented Mar 3, 2026

I managed to validate this on a local instance by downgrading to tc-admin 4.0.0. Please merge for me when you're ready :)

Copy link
Contributor

@matt-boris matt-boris left a comment

Choose a reason for hiding this comment

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

👍🏻

@matt-boris matt-boris merged commit 2a79222 into taskcluster:main Mar 3, 2026
73 checks passed
@ahal ahal deleted the ahal/push-xuxzmrtlpskq branch March 4, 2026 14:49
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants