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

"Purge workers cache" button on TaskInfo component#56

Merged
jonasfj merged 5 commits into
taskcluster:masterfrom
garbas:feature/1250414-purge-workers-cache-button
Feb 24, 2016
Merged

"Purge workers cache" button on TaskInfo component#56
jonasfj merged 5 commits into
taskcluster:masterfrom
garbas:feature/1250414-purge-workers-cache-button

Conversation

@garbas
Copy link
Copy Markdown
Contributor

@garbas garbas commented Feb 23, 2016

This is still WIP but I would need some feedback on the code since this is my first contribution.

Also any advise how I would test this would be more then welcome. Problem with testing is that I need to login with this credentials -> https://tools.taskcluster.net/auth/clients/#garbas

I went as far as manually creating credentials record in localStorage, but without luck (for now).

@djmitche
Copy link
Copy Markdown
Contributor

You can add a set of credentials by copy/paste with the ill-named "Preferences" tool.

Comment thread lib/ui/taskinfo.jsx Outdated
task: this.queue.task(this.props.status.taskId)
task: task,
selectedCacheNames: new Promise(function(resolve, reject) {
var resolved = Promise.resolve(task).then(
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

If I understand correctly, this is forcing a wait for the resolution of the task promise. I don't think that's necessary -- I believe load resolves any promises for you. So you could use

selectedCacheNames: task.then((t) => _.keys(self.getCacheNames(t)))

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

ah right ... need to remind myself i'm in ES6 context :)

@djmitche
Copy link
Copy Markdown
Contributor

I think this is the right design -- @jonasfj?

@garbas
Copy link
Copy Markdown
Contributor Author

garbas commented Feb 23, 2016

@djmitche i used garbas clientId you gave me and i get
2016-02-23-190814_638x701_scrot

should i try to find another task to test or i need bigger scope for my clientId?

Comment thread lib/ui/taskinfo.jsx Outdated
</label>
</li>;
})}
</ul>
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I think it's better to make sub-component called PurgeCacheButton with the property cacheNames...

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Okay, you would need provisionerId and workerType as properties too...

@djmitche
Copy link
Copy Markdown
Contributor

It looks like Jonas fixed your permissions issue?

@jonasfj
Copy link
Copy Markdown
Contributor

jonasfj commented Feb 23, 2016

Yeah, I fixed the scope issue...

IMO can we just give purge-cache:* to releng, relops etc... So we don't have to use perma creds.

@djmitche
Copy link
Copy Markdown
Contributor

Hold off on that until we namespace the workerTypes, and we can grant more specific scopes

@garbas
Copy link
Copy Markdown
Contributor Author

garbas commented Feb 24, 2016

@djmitche permission was fixed and i could tested that purgeCache worked.
@jonasfj i fixed code based on your comments

one thing i discovered (as i was testing) is that once you open ConfirmAction dialog you need to reload the page to reuse the same button. i fixed this by resetting initial state when result from action comes back 967b53f

@jonasfj
Copy link
Copy Markdown
Contributor

jonasfj commented Feb 24, 2016

r+ shipping...

jonasfj added a commit that referenced this pull request Feb 24, 2016
…e-button

"Purge workers cache" button on TaskInfo component
@jonasfj jonasfj merged commit 515388c into taskcluster:master Feb 24, 2016
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants