LogDog: Use per-project settings for archival.

LogDog: Use per-project settings for archival.

Update the Archivist microservice to apply per-project configurations.
Log streams are now archived to the GS buckets that their project
configures. Project archival preferences are also honored.

BUG=

Committed: https://github.com/luci/luci-go/commit/78f20e92d581fc9fb6b1d9afa4466226e32f5fdd

[dnj (Google), 2016-05-11 16:08:42]

dnj@google.com changed reviewers:
+ dnj@google.com, hinoka@chromium.org, iannucci@chromium.org

[dnj (Google), 2016-05-11 16:08:42]

PTAL. This updates the Archivist to load per-project settings.

[dnj, 2016-05-17 20:35:02]

Description was changed from

==========
LogDog: Use per-project settings for archival.

BUG=
==========

to

==========
LogDog: Use per-project settings for archival.

Update the Archivist microservice to apply per-project configurations.
Log streams are now archived to the GS buckets that their project
configures. Project archival preferences are also honored.

BUG=
==========

[nodir, 2016-05-19 15:56:28]

nodir@chromium.org changed reviewers:
+ nodir@chromium.org

[nodir, 2016-05-19 15:56:29]

I didn't finish reviewing because this CL needs a rebase

https://codereview.chromium.org/1968063003/diff/1/server/cmd/logdog_archivist...
File server/cmd/logdog_archivist/main.go (right):

https://codereview.chromium.org/1968063003/diff/1/server/cmd/logdog_archivist...
server/cmd/logdog_archivist/main.go:228: GSStagingBase:
gs.Path(acfg.GsStagingBase),
I assume this will be rebased to be use GsStagingBucket or something like that

https://codereview.chromium.org/1968063003/diff/1/server/cmd/logdog_archivist...
server/cmd/logdog_archivist/main.go:232: IndexByteRange:  
int(acfg.IndexByteRange),
I assume this will be rebased to be use GsStagingBucket and ArchiveIndexConfig
fields

https://codereview.chromium.org/1968063003/diff/1/server/cmd/logdog_archivist...
server/cmd/logdog_archivist/main.go:242: }
return err?

https://codereview.chromium.org/1968063003/diff/1/server/cmd/logdog_archivist...
server/cmd/logdog_archivist/main.go:249: if r := pcfg.ArchiveIndexStreamRange; r
>= 0 {
I still don't get this obsession to save a field value to a variable. It is more
code and ~same speed (maybe more). I am under impression that you are doing this
only because Golang has this assignment-in-if feature, it is abuse

https://codereview.chromium.org/1968063003/diff/1/server/internal/logdog/arch...
File server/internal/logdog/archivist/archivist.go (right):

https://codereview.chromium.org/1968063003/diff/1/server/internal/logdog/arch...
server/internal/logdog/archivist/archivist.go:107: // Settings retrieve the
settings for an archival.
s/retrieve/store
SettingsLoader retrieves them; Settings does not have methods

https://codereview.chromium.org/1968063003/diff/1/server/internal/logdog/arch...
server/internal/logdog/archivist/archivist.go:114: GSStagingBase gs.Path
rebase?

https://codereview.chromium.org/1968063003/diff/1/server/internal/logdog/arch...
server/internal/logdog/archivist/archivist.go:116: // AlwaysCreateBinary if
true, means that a binary should be archived
nit: comma before if?

https://codereview.chromium.org/1968063003/diff/1/server/internal/logdog/arch...
server/internal/logdog/archivist/archivist.go:385: panic("no settings loader
configured")
I am glad to see you didn't ban panics altogether ;)

[dnj (Google), 2016-05-19 16:34:52]

https://codereview.chromium.org/1968063003/diff/1/server/cmd/logdog_archivist...
File server/cmd/logdog_archivist/main.go (right):

https://codereview.chromium.org/1968063003/diff/1/server/cmd/logdog_archivist...
server/cmd/logdog_archivist/main.go:228: GSStagingBase:
gs.Path(acfg.GsStagingBase),
On 2016/05/19 15:56:29, nodir wrote:
> I assume this will be rebased to be use GsStagingBucket or something like that

Yeah. This is weird, I did a "dependency upload" and it should have uploaded the
rebased version. I uploaded again; it is using that in the current patch set.

https://codereview.chromium.org/1968063003/diff/1/server/cmd/logdog_archivist...
server/cmd/logdog_archivist/main.go:232: IndexByteRange:  
int(acfg.IndexByteRange),
On 2016/05/19 15:56:29, nodir wrote:
> I assume this will be rebased to be use GsStagingBucket and ArchiveIndexConfig
> fields

Acknowledged.

https://codereview.chromium.org/1968063003/diff/1/server/cmd/logdog_archivist...
server/cmd/logdog_archivist/main.go:242: }
On 2016/05/19 15:56:29, nodir wrote:
> return err?

er um ... yes.

https://codereview.chromium.org/1968063003/diff/1/server/cmd/logdog_archivist...
server/cmd/logdog_archivist/main.go:249: if r := pcfg.ArchiveIndexStreamRange; r
>= 0 {
On 2016/05/19 15:56:29, nodir wrote:
> I still don't get this obsession to save a field value to a variable. It is
more
> code and ~same speed (maybe more). I am under impression that you are doing
this
> only because Golang has this assignment-in-if feature, it is abuse

In this case, I think it is less wordy and therefore more readable. I would not
be doing this if Go didn't have assignment-in-if, since it would be less
readable. I don't think this is abuse at all.

https://codereview.chromium.org/1968063003/diff/1/server/internal/logdog/arch...
File server/internal/logdog/archivist/archivist.go (right):

https://codereview.chromium.org/1968063003/diff/1/server/internal/logdog/arch...
server/internal/logdog/archivist/archivist.go:107: // Settings retrieve the
settings for an archival.
On 2016/05/19 15:56:29, nodir wrote:
> s/retrieve/store
> SettingsLoader retrieves them; Settings does not have methods

Done.

https://codereview.chromium.org/1968063003/diff/1/server/internal/logdog/arch...
server/internal/logdog/archivist/archivist.go:114: GSStagingBase gs.Path
On 2016/05/19 15:56:29, nodir wrote:
> rebase?

Acknowledged.

https://codereview.chromium.org/1968063003/diff/1/server/internal/logdog/arch...
server/internal/logdog/archivist/archivist.go:116: // AlwaysCreateBinary if
true, means that a binary should be archived
On 2016/05/19 15:56:29, nodir wrote:
> nit: comma before if?

Done.

https://codereview.chromium.org/1968063003/diff/1/server/internal/logdog/arch...
server/internal/logdog/archivist/archivist.go:385: panic("no settings loader
configured")
On 2016/05/19 15:56:29, nodir wrote:
> I am glad to see you didn't ban panics altogether ;)

IMO panics have their place. I think explicit error handling is generally
better, though.

[nodir, 2016-05-19 17:27:49]

lgtm

https://codereview.chromium.org/1968063003/diff/60001/server/internal/logdog/...
File server/internal/logdog/archivist/archivist.go (right):

https://codereview.chromium.org/1968063003/diff/60001/server/internal/logdog/...
server/internal/logdog/archivist/archivist.go:129: // IndexStreamRange is the
maximum number of stream indexes in between index
stream entries? below too

[dnj (Google), 2016-05-19 22:53:55]

https://codereview.chromium.org/1968063003/diff/60001/server/internal/logdog/...
File server/internal/logdog/archivist/archivist.go (right):

https://codereview.chromium.org/1968063003/diff/60001/server/internal/logdog/...
server/internal/logdog/archivist/archivist.go:129: // IndexStreamRange is the
maximum number of stream indexes in between index
On 2016/05/19 17:27:49, nodir wrote:
> stream entries? below too

Index entries. So basically all three of these determine the same index
frequency. Whichever trips first makes an index entry happen.

[dnj (Google), 2016-05-20 01:13:41]

The CQ bit was checked by dnj@google.com

[dnj (Google), 2016-05-20 01:13:41]

The patchset sent to the CQ was uploaded after l-g-t-m from nodir@chromium.org
Link to the patchset: https://codereview.chromium.org/1968063003/#ps40002
(title: "Updated patchset dependency")

[commit-bot: I haz the power, 2016-05-20 01:13:47]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1968063003/40002
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1968063003/40002

[commit-bot: I haz the power, 2016-05-20 01:20:43]

Description was changed from

==========
LogDog: Use per-project settings for archival.

Update the Archivist microservice to apply per-project configurations.
Log streams are now archived to the GS buckets that their project
configures. Project archival preferences are also honored.

BUG=
==========

to

==========
LogDog: Use per-project settings for archival.

Update the Archivist microservice to apply per-project configurations.
Log streams are now archived to the GS buckets that their project
configures. Project archival preferences are also honored.

BUG=

Committed:
https://github.com/luci/luci-go/commit/78f20e92d581fc9fb6b1d9afa4466226e32f5fdd
==========

[commit-bot: I haz the power, 2016-05-20 01:20:44]

Committed patchset #5 (id:40002) as
https://github.com/luci/luci-go/commit/78f20e92d581fc9fb6b1d9afa4466226e32f5fdd