New issue
Advanced search Search tips

Issue 751872 link

Starred by 1 user

Issue metadata

Status: Fixed
Owner:
Closed: Oct 2017
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: ----
Pri: 1
Type: Bug



Sign in to add a comment

LogDog: Remove "list" endpoint and support.

Project Member Reported by d...@chromium.org, Aug 2 2017

Issue description

The "list" endpoint was really useful when initially setting up LogDog. However, the log stream space has grown too vast for it to be practically useful. Remove the "list" endpoint removes a lot of complicated code and also reduces the datastore cost of new streams.

There was a dysfunctional list UI in the LogDog app. In order to avoid "regression" (in quotes, b/c the UI and list functionality were both not useful), replace it with a query UI. The query UI will be basic at first, but it should be trivial to improve it in future iterations.

There are no known clients of the list endpoint.
 
Project Member

Comment 1 by bugdroid1@chromium.org, Aug 2 2017

The following revision refers to this bug:
  https://chromium.googlesource.com/external/github.com/luci/luci-go.git/+/2dee6bcd09b0536f4e7c1a3a41331b60bcb184e0

commit 2dee6bcd09b0536f4e7c1a3a41331b60bcb184e0
Author: dnj <dnj@chromium.org>
Date: Wed Aug 02 23:14:55 2017

[logdog] Stream view explicitly cancels request.

Currently, a request, when cancelled, is ignored, but is still alllowed
to proceed. This causes HTTP requests for logs that are cancelled to
actually cancel their underlying requests.

BUG= chromium:751872 
TEST=None

Review-Url: https://codereview.chromium.org/2989313002

[modify] https://crrev.com/2dee6bcd09b0536f4e7c1a3a41331b60bcb184e0/web/inc/logdog-stream-view/fetcher.ts
[modify] https://crrev.com/2dee6bcd09b0536f4e7c1a3a41331b60bcb184e0/web/inc/logdog-stream-view/model.ts
[modify] https://crrev.com/2dee6bcd09b0536f4e7c1a3a41331b60bcb184e0/web/inc/logdog-stream-view/query.ts
[modify] https://crrev.com/2dee6bcd09b0536f4e7c1a3a41331b60bcb184e0/web/inc/logdog-stream/client.ts

Project Member

Comment 2 by bugdroid1@chromium.org, Aug 3 2017

The following revision refers to this bug:
  https://chromium.googlesource.com/external/github.com/luci/luci-go.git/+/6ad55f6eef0b637bacc1782dc260803facb1b021

commit 6ad55f6eef0b637bacc1782dc260803facb1b021
Author: dnj <dnj@chromium.org>
Date: Thu Aug 03 00:15:07 2017

[logdog] Replace list view with query view.

The list view is near-useless, as the log stream space is too large to
effectively navigate through list clicks. Replace it with a query view.

This query view is fairly simple at the moment. The intent is to improve
it over time.

BUG= chromium:751872 
TEST=local

Review-Url: https://codereview.chromium.org/2991253003

[modify] https://crrev.com/6ad55f6eef0b637bacc1782dc260803facb1b021/web/apps/logdog-app/elements/elements.html
[modify] https://crrev.com/6ad55f6eef0b637bacc1782dc260803facb1b021/web/apps/logdog-app/elements/logdog-app/logdog-app.html
[modify] https://crrev.com/6ad55f6eef0b637bacc1782dc260803facb1b021/web/apps/logdog-app/elements/routing.html
[modify] https://crrev.com/6ad55f6eef0b637bacc1782dc260803facb1b021/web/inc/apps/logdog-app/main.ts
[delete] https://crrev.com/e74d1c071c854fc4b50e6d980f03164b03cdc61a/web/inc/logdog-list-view/logdog-list-view.html
[add] https://crrev.com/6ad55f6eef0b637bacc1782dc260803facb1b021/web/inc/logdog-query-view/logdog-query-panel.html
[add] https://crrev.com/6ad55f6eef0b637bacc1782dc260803facb1b021/web/inc/logdog-query-view/logdog-query-view.html
[add] https://crrev.com/6ad55f6eef0b637bacc1782dc260803facb1b021/web/inc/logdog-query-view/view.ts
[modify] https://crrev.com/6ad55f6eef0b637bacc1782dc260803facb1b021/web/inc/logdog-stream-view/logdog-stream-view.html

Project Member

Comment 3 by bugdroid1@chromium.org, Aug 3 2017

The following revision refers to this bug:
  https://chromium.googlesource.com/external/github.com/luci/luci-go.git/+/31b62d1f3adcb115016e445192828829ae3ef081

commit 31b62d1f3adcb115016e445192828829ae3ef081
Author: dnj <dnj@chromium.org>
Date: Thu Aug 03 00:38:17 2017

[logdog] Remove list functionality.

LogDog has grown too large for this to be useful. List has been replaced
in the UI with Query, which is superior for all practical purposes. Now,
remove List entirely to reduce code burden.

BUG= chromium:751872 
TEST=Non

Review-Url: https://codereview.chromium.org/2991253004

[modify] https://crrev.com/31b62d1f3adcb115016e445192828829ae3ef081/logdog/api/endpoints/coordinator/logs/v1/logs.pb.go
[modify] https://crrev.com/31b62d1f3adcb115016e445192828829ae3ef081/logdog/api/endpoints/coordinator/logs/v1/logs.proto
[modify] https://crrev.com/31b62d1f3adcb115016e445192828829ae3ef081/logdog/api/endpoints/coordinator/logs/v1/logsserver_dec.go
[modify] https://crrev.com/31b62d1f3adcb115016e445192828829ae3ef081/logdog/api/endpoints/coordinator/logs/v1/pb.discovery.go
[delete] https://crrev.com/5e63e25d0fb4fdcc6adc8afceb24a9244f06c724/logdog/appengine/coordinator/endpoints/logs/list.go
[delete] https://crrev.com/5e63e25d0fb4fdcc6adc8afceb24a9244f06c724/logdog/appengine/coordinator/endpoints/logs/list_test.go
[modify] https://crrev.com/31b62d1f3adcb115016e445192828829ae3ef081/logdog/appengine/coordinator/endpoints/registration/registerPrefix.go
[modify] https://crrev.com/31b62d1f3adcb115016e445192828829ae3ef081/logdog/appengine/coordinator/endpoints/registration/registerPrefix_test.go
[modify] https://crrev.com/31b62d1f3adcb115016e445192828829ae3ef081/logdog/appengine/coordinator/endpoints/services/registerStream.go
[modify] https://crrev.com/31b62d1f3adcb115016e445192828829ae3ef081/logdog/appengine/coordinator/endpoints/services/registerStream_test.go
[delete] https://crrev.com/5e63e25d0fb4fdcc6adc8afceb24a9244f06c724/logdog/appengine/coordinator/hierarchy/component.go
[delete] https://crrev.com/5e63e25d0fb4fdcc6adc8afceb24a9244f06c724/logdog/appengine/coordinator/hierarchy/componentID.go
[delete] https://crrev.com/5e63e25d0fb4fdcc6adc8afceb24a9244f06c724/logdog/appengine/coordinator/hierarchy/component_test.go
[delete] https://crrev.com/5e63e25d0fb4fdcc6adc8afceb24a9244f06c724/logdog/appengine/coordinator/hierarchy/hierarchy.go
[delete] https://crrev.com/5e63e25d0fb4fdcc6adc8afceb24a9244f06c724/logdog/appengine/coordinator/hierarchy/hierarchy_test.go
[delete] https://crrev.com/5e63e25d0fb4fdcc6adc8afceb24a9244f06c724/logdog/appengine/coordinator/hierarchy/project.go
[modify] https://crrev.com/31b62d1f3adcb115016e445192828829ae3ef081/logdog/client/cli/main.go
[delete] https://crrev.com/5e63e25d0fb4fdcc6adc8afceb24a9244f06c724/logdog/client/cli/subcommandList.go
[modify] https://crrev.com/31b62d1f3adcb115016e445192828829ae3ef081/logdog/client/coordinator/client_test.go
[delete] https://crrev.com/5e63e25d0fb4fdcc6adc8afceb24a9244f06c724/logdog/client/coordinator/list.go

Oops, we were actually using this. :(

https://cs.chromium.org/chromium/src/tools/accessibility/rebase_dump_accessibility_tree_test.py?q=%22cit+logdog+ls%22&l=123

Can we achieve the same thing using "query"?

Looks like this is the root of issue 760245
We use this in src/tools/accessibility/rebase_dump_accessibility_tree_test.py
Is there an alternative query command we can use?
We are working on a solution with query.

Comment 7 by d...@chromium.org, Oct 20 2017

Status: Fixed (was: Started)

Sign in to add a comment