1637912 - TC: Mismatch in the documentation on "_" vs "-"

Status: RESOLVED WONTFIX

(Firefox Build System :: Task Configuration, defect)

Description

[Sylvestre Ledru [:Sylvestre]]

The documentation references "run_on_projects" ( https://firefox-source-docs.mozilla.org/taskcluster/attributes.html#run-on-projects ) but in the yaml configuration, we use
run-on-projects ( https://searchfox.org/mozilla-central/source/taskcluster/ci/addon/kind.yml#22 )

This is causing the sphinx search return garbage when searching with run-on-projects
https://firefox-source-docs.mozilla.org/search.html?q=run-on-projects&check_keywords=yes&area=default
with "_" instead, it is providing me the right information.
https://firefox-source-docs.mozilla.org/search.html?q=run_on_projects&check_keywords=yes&area=default#

Comment 1

[Tom Prince [:tomprince]]

This issues derives from the fact that run-on-projects is a key in the task definition, which use kebab-case, but the documentation exists only for that attribute that key causes to be set, and attributes are historically and mostly snake_case.

There are a couple of things we could do:

• we should document the keys various transforms take directly (taskgraph.transforms.task:transform at a minimum). There are a bunch of in-code comments that do this to an extent, and the schema library we use allows adding inspectable metadata for things[1], that we could turn into documentation.
• we could change the attributes to use kebab-case consistently; this would definitely be a large undertaking, as we'd want to do adjust it for all the out-of-tree taskgraph using projects at the same time. I'm not sure if there any consumers outside of taskgraph, currently, that would need to be changed as well; attributes don't end up in task definitions, other than in the artifacts the decision task generates, but I'm not sure what might be consuming them. I'd also want to look at parameters (which are also snake case) when looking at this, which is consumed in a bunch of out-of-tree places.

[1] This was added long after we started using the library, so the use of the metadata is inconsistent.

Comment 2

[Sylvestre Ledru [:Sylvestre]]

Tom, do you know who can fix that ? you ?
thanks

Comment 3

[Tom Prince [:tomprince]]

Fixing this is probably a several weeks to a couple of months project, so it would need to be scheduled accordingly.

Comment 4

[Sylvestre Ledru [:Sylvestre]]

Well, I am just talking about fixing the doc. The underlying issue can be fixed later.

Comment 5

[Tom Prince [:tomprince]]

I'm not entirely sure what you are suggesting, since the documentation is correct for the current state of affairs.

Comment 6

[Aki Sasaki (not active)]

It looks like sphinx treats - as a word separator, so searching for run-on-projects searches for run, on, and projects. This is poor. I'm not sure if we can adjust how sphinx performs searches; if so, that may be a straightforward solution. If we can't fix sphinx's search behavior, then moving everything to kebab-case will likely break all searches using kebab-case terms.

Alternate solution: could we remove the kebab-case restriction? Namely:

1. allow for both kebab-case and snake_case in the yml
2. continue to translate kebab-case to snake_case in the transforms; we should detect and fail if we have conflicting config-key and config_key set
3. change the yml to use snake_case over time
4. when everything is in snake_case, disallow kebab-case and remove the kebab-case checks from (2).

We may want to figure out why kebab-case was required in the first place, in case there are reasons to keep it that I'm not thinking of.

[edit] if we're changing sphinx search, then regexes or globs may help here. e.g. run.on.projects or run?on?projects or the like; or we could treat - and _ as equivalent in the search.

Comment 7

[Andrew Halberstadt [:ahal]]

Since comment 6 the search has been overhauled, so this should be better now? But I agree with Tom. While I agree it's confusing, the current docs are technically correct. I think the main issue is that run-on-projects the key that tasks can use is actually undocumented. Though this should now go in the Taskgraph docs.