Skip to content

Conversation

@jcristau
Copy link
Contributor

Fixes #493

@jcristau jcristau force-pushed the template-versioned-decision branch from e68cdd2 to 6286086 Compare June 25, 2024 15:09
@@ -1,5 +1,6 @@
{
"project_name": "my-project",
"taskgraph_version": "{{ cookiecutter.taskgraph_version }}",
Copy link
Collaborator

@ahal ahal Jun 25, 2024

Choose a reason for hiding this comment

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

I don't think this will work. The template context comes from the other variables in this file, so this seems like it would lead to some kind of recursion error?

Instead this is supposed to contain the default value we use for this key in case cookiecutter is ran with interactive mode disabled. That being said, I don't think we should prompt for this value anyway. Instead we should either use latest or the same version as what they are running taskgraph init with.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yeah I'm not sure, I was going from https://cookiecutter.readthedocs.io/en/stable/advanced/injecting_context.html which seems to be doing something similar?

Copy link
Collaborator

Choose a reason for hiding this comment

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

On second thought, we should definitely use the same version as what they ran taskgraph init with. Otherwise, the generated graph might fail inside the Decision task.

Copy link
Collaborator

Choose a reason for hiding this comment

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

Oh, I think I missed the extra context being passed in there. And thanks for the link.

@jcristau jcristau marked this pull request as ready for review June 25, 2024 16:52
@jcristau jcristau requested a review from ahal June 25, 2024 16:52
@@ -1,5 +1,6 @@
{
"project_name": "my-project",
"taskgraph_version": "{{ cookiecutter.taskgraph_version }}",
Copy link
Collaborator

Choose a reason for hiding this comment

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

Oh, I think I missed the extra context being passed in there. And thanks for the link.

@ahal ahal merged commit 91ac1a1 into taskcluster:main Jun 26, 2024
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.

taskgraph init should use version pinned decision image and no requirements file

2 participants