Skip to content
This repository was archived by the owner on Jul 18, 2019. It is now read-only.

Bug 1466679 - use rootUrl in 'taskcluster signin'#192

Merged
djmitche merged 2 commits into
taskcluster:masterfrom
djmitche:bug1466679
Jun 25, 2018
Merged

Bug 1466679 - use rootUrl in 'taskcluster signin'#192
djmitche merged 2 commits into
taskcluster:masterfrom
djmitche:bug1466679

Conversation

@djmitche
Copy link
Copy Markdown
Contributor

@djmitche djmitche commented Jun 4, 2018

No description provided.

@djmitche djmitche self-assigned this Jun 4, 2018
@djmitche djmitche requested a review from petemoore June 4, 2018 21:03
@djmitche
Copy link
Copy Markdown
Contributor Author

djmitche commented Jun 4, 2018

I'm not sure why the first commit changed anything in vendor.json. Running make updated those hashes. I can revert that file if it's the right thing to do.

@djmitche
Copy link
Copy Markdown
Contributor Author

djmitche commented Jun 4, 2018

The test failure is because the "1000 year task" apparently only had a 1-year artifact expiration. I've started another (https://tools.taskcluster.net/groups/SmJVXbZERxigZVunstEveQ/tasks/SmJVXbZERxigZVunstEveQ/details) and will land a test fix if that works.

@djmitche
Copy link
Copy Markdown
Contributor Author

djmitche commented Jun 5, 2018

Rebased over the test fix

@petemoore
Copy link
Copy Markdown
Member

petemoore commented Jun 6, 2018

I'll have a look at this tomorrow!

@djmitche
Copy link
Copy Markdown
Contributor Author

djmitche commented Jun 11, 2018

@petemoore ping?

@petemoore
Copy link
Copy Markdown
Member

petemoore commented Jun 25, 2018

I'm looking at this now! :-)

petemoore
petemoore approved these changes Jun 25, 2018
View reviewed changes
Copy link
Copy Markdown
Member

@petemoore petemoore left a comment •
edited
Loading

Choose a reason for hiding this comment

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

This looks great.
I really like that:

  1. the clients auto-delete on expiry
  2. a graceful server is used (note, these days this can be done in the standard library, so graceful.v1 probably isn't needed any longer)
  3. multiple clients are supported
  4. that a callback is used to generate the export statements

A question: why do we involve the web client rather than just making the API calls to create the client directly from taskcluster-cli? Is that to make it easier to customise scopes? Or is it so the credentials are in local storage so that web sessions benefit from the credentials too?

@djmitche
Copy link
Copy Markdown
Contributor Author

djmitche commented Jun 25, 2018

Thanks! We use the web client because it's not possible to sign in with Auth0 without a browser.

I don't see a reference to graceful here -- I didn't write the rest of the signin command, so maybe that was someone else.

@djmitche djmitche merged commit 9002b13 into taskcluster:master Jun 25, 2018
This was referenced Jul 10, 2018
Closed
Closed
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Reviewers

1 more reviewer

@petemoore petemoore petemoore approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

@djmitche djmitche

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

2 participants

@djmitche @petemoore