New issue
Advanced search Search tips
Note: Color blocks (like or ) mean that a user may not be available. Tooltip shows the reason.

Issue 806292 link

Starred by 1 user

Issue metadata

Status: Fixed
Owner:
Closed: Feb 2018
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: ----
Pri: 3
Type: Bug

Blocking:
issue 788034



Sign in to add a comment

bqschemaupdater: -dry-run output is not useful

Project Member Reported by no...@chromium.org, Jan 26 2018

Issue description

context: https://chromium-review.googlesource.com/c/chromium/tools/build/+/888179

When running

  bqschemaupdater -dry-run -table cr-buildbucket-dev.test.x \
    -message devtools_goma.CompileEvent \
    -I ~/infra/goma-client/lib/

in CWD = local of https://chromium.googlesource.com/chromium/tools/build/+/master/scripts/slave/goma


The output is

============
Running getTableMetadata for datasetID test tableID x
googleapi: Error 404: Not found: Dataset cr-buildbucket-dev:test, notFound
Would run createTable with datasetID test, tableID x, metadata &{Name: Description:This is BigQuery schema for goma-related events stored in chrome-infra.
NEXT ID TO USE: 6 Schema:[0xc42008da90 0xc42008dae0 0xc42008db30 0xc4202865f0 0xc420286640] ViewQuery: UseLegacySQL:false UseStandardSQL:false TimePartitioning:0xc42039a080 ExpirationTime:0001-01-01 00:00:00 +0000 UTC Labels:map[] ExternalDataConfig:<nil> FullID: Type: CreationTime:0001-01-01 00:00:00 +0000 UTC LastModifiedTime:0001-01-01 00:00:00 +0000 UTC NumBytes:0 NumRows:0 StreamingBuffer:<nil> ETag:}
2018/01/26 09:34:36 Finished updating table.
============

This is hard to read.

Further, let's imagine we have a table X with 100 fields and we are adding one more field to the proto. Dry run will print 101 fields and that would be hard to read and generally hard to understand what exactly is being changed.

 

Comment 1 by no...@chromium.org, Jan 26 2018

I believe we could solve both problems and simplify CLI by

- when not in dry-run mode, compute schema diff between new schema and actual schema
- exit if no diff
- print the diff in a nice format and ask for use confirmation
- if confirmed, make the changes

and then, we don't need -dry-run flag. And running it without dry-run gives more confidence what exactly is gonna happen. WDYT?

Comment 2 by no...@chromium.org, Jan 31 2018

Cc: katthomas@google.com
Thanks for documenting this nodir!

Comment 4 by no...@chromium.org, Jan 31 2018

WDYT about the proposal in comment 1?

Comment 5 by no...@chromium.org, Feb 22 2018

Owner: no...@chromium.org
Status: Started (was: Untriaged)
going to do this because going to use this tool myself for a large schema
ty! Also, sorry for not responding to #4. SGTM.

Comment 7 by no...@chromium.org, Feb 22 2018

Blocking: 788034
Project Member

Comment 10 by bugdroid1@chromium.org, Feb 23 2018

The following revision refers to this bug:
  https://chromium.googlesource.com/infra/luci/luci-go.git/+/934d5f0d282a9be41f0ef85bc3d5dda31cd58eb0

commit 934d5f0d282a9be41f0ef85bc3d5dda31cd58eb0
Author: Nodir Turakulov <nodir@google.com>
Date: Fri Feb 23 21:43:12 2018

[bqschemaupdater] add confirmation and diff

When creating a new table, print schema.
When updating an existing table, print unified diff of the schema.
In both cases, ask for a user confirmation before making actual changes.

Use fmt.Print* functions directly instead of log.Print* functions.

This change makes -dry-run flag much less useful.
It will be deleted in a separate CL.

Bug:  806292 
Change-Id: I67c8f9655ad6ee55b2adb36ac3e4c60c6facba0a
Reviewed-on: https://chromium-review.googlesource.com/933302
Commit-Queue: Nodir Turakulov <nodir@chromium.org>
Reviewed-by: Katie Thomas <katthomas@google.com>

[modify] https://crrev.com/934d5f0d282a9be41f0ef85bc3d5dda31cd58eb0/tools/cmd/bqschemaupdater/dry_run_table_store.go
[modify] https://crrev.com/934d5f0d282a9be41f0ef85bc3d5dda31cd58eb0/tools/cmd/bqschemaupdater/main.go
[modify] https://crrev.com/934d5f0d282a9be41f0ef85bc3d5dda31cd58eb0/tools/cmd/bqschemaupdater/main_test.go
[modify] https://crrev.com/934d5f0d282a9be41f0ef85bc3d5dda31cd58eb0/tools/cmd/bqschemaupdater/schema.go

Comment 11 by no...@chromium.org, Feb 23 2018

Status: Fixed (was: Started)

Sign in to add a comment