Anthony Bishopric anthonybishopric

Organizations

@platypus-platform
anthonybishopric commented on pull request square/p2#158
@anthonybishopric

Is there any way to rewrite this loop such that it doesn't guarantee an added 500ms to the test suite?

anthonybishopric commented on pull request square/p2#158
@anthonybishopric

The maximum number ofconcurrent updates permitted by this order is active. This was really confusing. You mean "the parameter active designates th…

anthonybishopric commented on pull request square/p2#157
@anthonybishopric

Looks good, although I humbly request that we keep the commit history pristine - it makes looking at small updates easier without squashing.

anthonybishopric commented on pull request square/p2#158
@anthonybishopric

Why kill the README? It's nice to have per-package docs, or if you are replacing with a wiki page at least link to it.

anthonybishopric commented on pull request square/p2#158
@anthonybishopric

Why just this channel?

anthonybishopric commented on pull request square/p2#158
@anthonybishopric

What happens if lock.Renew() is called after lock.Destroy()? Wondering about the ordering of the defer and this goroutine.

anthonybishopric closed pull request square/p2#144
@anthonybishopric
Add artifact cacher
anthonybishopric commented on pull request square/p2#157
@anthonybishopric

Left one more question in there, otherwise :+1:

anthonybishopric commented on pull request square/p2#157
@anthonybishopric

Does url.Parse correctly return the empty string for the scheme if it's just a file path?

anthonybishopric commented on pull request square/p2#157
@anthonybishopric

This is a neat simplification. Keep in mind that we want some strategy to allow caching across multiple pods if we share launchables. A shared jdk …

anthonybishopric commented on pull request square/p2#156
@anthonybishopric

Please verify with the integration test (and it looks like Travis is broken too) but otherwise :+1:

anthonybishopric commented on pull request square/p2#156
@anthonybishopric

That change seems sane. It should be easy to alter the only caller to pod.Verify()

anthonybishopric commented on pull request square/p2#156
@anthonybishopric

:+1:

anthonybishopric commented on pull request square/p2#156
@anthonybishopric

In that case, I would suggest making an auth.Manifest interface. I'd like to be able to write an authorization policy access the structured fields …

anthonybishopric commented on pull request square/p2#156
@anthonybishopric

s/manifest/pod ID/

anthonybishopric commented on pull request square/p2#156
@anthonybishopric

we already have util.StackError, you may or may not wish to use that

anthonybishopric commented on pull request square/p2#156
@anthonybishopric

would it make more sense to pass a pods.Manifest?

anthonybishopric commented on pull request square/p2#156
@anthonybishopric

I think there's a case for using old keyring. We don't want there to be a strict online dependency with the keyserver - especially if it's a countd…

anthonybishopric commented on pull request square/p2#156
@anthonybishopric

Although the return type of this function is still openpgp.Keyring. Not sure how that is working

anthonybishopric commented on pull request square/p2#156
@anthonybishopric

You only use curly braces when initializing a struct, iirc

anthonybishopric commented on pull request square/p2#156
@anthonybishopric

I'm into it. :+1:

anthonybishopric commented on pull request square/p2#156
@anthonybishopric

auth.SigVerifier is an interface, so it's actually much worse than Mike points out - https://groups.google.com/forum/#!topic/golang-nuts/QzVDKv7p0Vs

@anthonybishopric
anthonybishopric commented on pull request square/p2#156
@anthonybishopric

Yeah it's not obvious from the channel abstraction, but our Consul watches are basically in a long poll against /intent/<hostname>/?recurse, which …

anthonybishopric commented on pull request square/p2#156
@anthonybishopric

Well, currently the signatures for all manifests in the intent store are verified every 5 minutes or when any manifests are scheduled. So you're gu…

anthonybishopric commented on pull request square/p2#156
@anthonybishopric

reading the keyring everytime will work well for guaranteeing freshness, but we'll probably want a way to do reasonable caching / modification dete…

anthonybishopric commented on pull request square/p2#155
@anthonybishopric

:+1:

anthonybishopric commented on pull request square/p2#155
@anthonybishopric

Nice cleanup! Make sure to run rake integration to ensure that the end-to-end test passes.