Skip to content

Remove TableInfo hierarchy, add TableType hierarchy#600

Merged
aozarov merged 5 commits intogoogleapis:masterfrom
mziccard:bigquery-hierachies
Feb 2, 2016
Merged

Remove TableInfo hierarchy, add TableType hierarchy#600
aozarov merged 5 commits intogoogleapis:masterfrom
mziccard:bigquery-hierachies

Conversation

@mziccard
Copy link
Contributor

This PR removes TableInfo hierarchy and replaces it with a hierarchy on TableType. This change is needed to make functional Table extend TableInfo. Notes:

  • A type object (DefaultTableType, ExternalTableType or ViewType) must be provided to create a JobInfo object.
  • ExternalDataConfiguration has been removed in favore of ExternalTableType which is also used for defining external table sources in QueryJobConfiguration.

@mziccard mziccard added the api: bigquery Issues related to the BigQuery API. label Jan 29, 2016
@googlebot googlebot added the cla: yes This human has signed the Contributor License Agreement. label Jan 29, 2016

This comment was marked as spam.

@ajkannan
Copy link

2 BigQuery lint errors unrelated to this PR:

gcloud-java-bigquery/src/test/java/com/google/gcloud/bigquery/WriteChannelConfigurationTest.java:50: error: Line is longer than 100 characters (found 113).
gcloud-java-bigquery/src/test/java/com/google/gcloud/bigquery/WriteChannelConfigurationTest.java:109: error: Line is longer than 100 characters (found 110).

This comment was marked as spam.

@ajkannan
Copy link

Took a first pass, looks good! Just some minor comments from me.

@mziccard
Copy link
Contributor Author

mziccard commented Feb 1, 2016

Thanks for the pass @ajkannan I addressed all comments in 931b4d3

@mziccard mziccard force-pushed the bigquery-hierachies branch 2 times, most recently from 3474f3b to 04fbf41 Compare February 1, 2016 14:17
@mziccard mziccard force-pushed the bigquery-hierachies branch from 04fbf41 to 931b4d3 Compare February 1, 2016 16:21
@ajkannan
Copy link

ajkannan commented Feb 1, 2016

Looks good, just one new lint error:
gcloud-java-bigquery/src/test/java/com/google/gcloud/bigquery/ViewDefinitionTest.java:65: error: Line is longer than 100 characters (found 111).

Aside from this PR, I think it'd be cool if we figured out some way to link the snippets in the READMEs to code we could test in Travis (something like the snippets here: https://cloud.google.com/appengine/docs/java/gettingstarted/ui_and_code). That way we'd always know our snippets are up to date after changes. But in lieu of that for now, it'd be good to test that the snippets work when this is about to get merged in, or when the info and functional objects get merged (if that's going to happen sooner rather than later).

This comment was marked as spam.

This comment was marked as spam.

@mziccard
Copy link
Contributor Author

mziccard commented Feb 1, 2016

Looks good, just one new lint error:

fixed

Aside from this PR, I think it'd be cool if we figured out some way to link the snippets in the READMEs to code we could test in Travis (something like the snippets here: https://cloud.google.com/appengine/docs/java/gettingstarted/ui_and_code). That way we'd always know our snippets are up to date after changes. But in lieu of that for now, it'd be good to test that the snippets work when this is about to get merged in, or when the info and functional objects get merged (if that's going to happen sooner rather than later).

I totally agree but this is not easy to do. Snippets that we have now are simple end-to-end usage showcases (e.g. update a blob if it exists or create it if not). On the other hand our examples are more complex and command-line oriented to show the usage of each single functionality. Snippets could be part of IT tests but they will need changes anyway: asserts, data definitions, etc.

What we could do is creating a snippet (and possibly name of the service?) package in gcloud-java-examples where we put each snippet as a class+main with a meaningful name (e.g. CreateOrUpdateBlob). We can then reference those files before the snippet as in the example you provided. What do you think? (I think this deserves a dedicated issue).

@mziccard
Copy link
Contributor Author

mziccard commented Feb 1, 2016

Renamed to TableDefinition in 085903a

@mziccard mziccard force-pushed the bigquery-hierachies branch from 4ee4554 to 085903a Compare February 1, 2016 20:03

This comment was marked as spam.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

api: bigquery Issues related to the BigQuery API. cla: yes This human has signed the Contributor License Agreement.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants