1426398 - Switch from taskcluster-client to taskcluster-client-web

Status: RESOLVED DUPLICATE of bug 1395356

(Tree Management :: Treeherder: Frontend, defect, P2)

Description

[Ed Morley [:emorley]]

Treeherder's frontend is currently using taskcluster-client v2.5.4. The Renovate bot has opened a PR to update that to v3.1.1, however the tests are failing with:

Module not found: Error: Can't resolve 'net' in '/home/travis/build/mozilla/treeherder/node_modules/amqplib/lib'
(https://travis-ci.org/mozilla/treeherder/jobs/318961426#L716)

This is because the client is presuming it's running in a node.js environment where `net` is available, which isn't true in the browser (unless webpack has been told to polyfill/stub out the module).

Rather than resort to polyfilling, it looks like we should actually be using the newly created:
https://github.com/taskcluster/taskcluster-client-web

This client is much lighter (fewer transitive dependencies) and will work out of the box.

Eli/Brian:
(a) is the above correct?
(b) is it a drop-in replacement (here: https://github.com/mozilla/treeherder/blob/b37ef957a2282bcc7dd5d0d884df4b3c05718992/ui/js/services/taskcluster.js#L5) or will additional changes be required?

Comment 1

[Brian Stack [:bstack]]

(a) is the above correct?

I believe so! Sorry we didn't think to warn you earlier about this switch.

(b)

Eli would be the one to ask to be sure but I'm 99% certain that it is.

Comment 2

[Ed Morley [:emorley]]

Don't be sorry! I'm excited that there's now a lighter client we can use :-)

Comment 3

[GitHub Bugzilla PR Linker]

Attachment: Link to GitHub pull-request: https://github.com/mozilla/treeherder/pull/3074

Comment 4

[Ed Morley [:emorley]]

Hmm looks like it's not a drop-in replacement, there doesn't seem to be a way to set global credentials like before (since I'm getting `Error: T.config is not a function`), so will require a bit of a refactor of how Treeherder uses it, since we'll have to update the shared client rather than global state when the localstorage credentials change. Brian, I don't suppose you might be able to have a go at that at some point? I'm not really familiar with the TC-actions usage (or most of Treeherder's frontend in general tbh) so it would take me quite a bit longer :-)

Comment 5

[Brian Stack [:bstack]]

Oh, now that you mention that, I fear that the new client relies on being able to do auth0 stuff. Is that true, dustin?

Comment 6

[Dustin J. Mitchell [:dustin] (he/him)]

It does support that as well -- https://github.com/taskcluster/taskcluster-client-web#calling-api-endpoints

I suspect that's what you're doing and getting an error -- can you give more context for that, a stack trace perhaps?  I don't see /T\.config/ in lib/index.js.

Comment 7

[Ed Morley [:emorley]]

That was from a minified build (hence the `T`), the stack trace (and links to docs where the global config was mentioned) are here:
https://github.com/mozilla/treeherder/pull/3074#issuecomment-354313344

...however I don't think it's needed, since this seems to have been an intentional design change from taskcluster-client@2.x -- in that there is no longer a global config, and only per-client configs.

Comment 8

[Dustin J. Mitchell [:dustin] (he/him)]

Ah, I'm not sure what you mean by "global config" (env vars?) but you're right that I don't think tc-client-web supports any such thing.

Comment 9

[Ed Morley [:emorley]]

Not environment variables, the global default configuration:

"You can also configure default options globally using taskcluster.config(options), as follows:"
(on https://github.com/taskcluster/taskcluster-client/tree/v2.5.4#configuring-credentials)

Comment 10

[Dustin J. Mitchell [:dustin] (he/him)]

Ah, interesting, I had totally forgotten that feature..

Comment 11

[Ed Morley [:emorley]]

This is now being performed in bug 1395356 :-D