Remove TableInfo hierarchy, add TableType hierarchy #600

mziccard · 2016-01-29T19:05:30Z

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.

gcloud-java-bigquery/src/main/java/com/google/gcloud/bigquery/ExternalTableType.java

@@ -298,7 +305,7 @@ public boolean equals(Object obj) {
   *     of 10 GB maximum size across all URIs.
   * @param schema the schema for the external data
   * @param format the source format of the external data
-   * @return a builder for an ExternalDataConfiguration object given source URIs, schema and format
+   * @return a builder for an ExternalTableDefinition object given source URIs, schema and format


ajkannan · 2016-01-30T01:18:51Z

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).

gcloud-java-bigquery/src/test/java/com/google/gcloud/bigquery/ITBigQueryTest.java

    assertTrue(bigquery.delete(DATASET, tableName));
  }

  @Test
  public void testUpdateNonExistingTable() {
-    TableInfo tableInfo =
-        TableInfo.of(TableId.of(DATASET, "test_update_non_existing_table"), SIMPLE_SCHEMA);
+


ajkannan · 2016-01-30T01:57:10Z

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

mziccard · 2016-02-01T13:03:43Z

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

ajkannan · 2016-02-01T17:59:37Z

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).

gcloud-java-bigquery/src/main/java/com/google/gcloud/bigquery/BaseTableDefinition.java

+  public enum Type {
+    /**
+     * A normal BigQuery table. Instances of {@code BaseTableDefinition} for this type are
+     * implemented by {@link DefaultTableDefinition}.


mziccard · 2016-02-01T18:42:53Z

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 · 2016-02-01T19:31:52Z

Renamed to TableDefinition in 085903a

gcloud-java-bigquery/src/main/java/com/google/gcloud/bigquery/BigQuery.java

-     * used to specify only the fields of interest. {@link BaseTableInfo#tableId()} and
-     * {@link BaseTableInfo#type()} are always returned, even if not specified.
+     * used to specify only the fields of interest. {@link TableInfo#tableId()} and
+     * {@link TableInfo#definition()} are always returned, even if not specified.


gcloud-java-bigquery/src/main/java/com/google/gcloud/bigquery/TableDefinition.java

+
+    @Override
+    public String toString() {
+      return MoreObjects.toStringHelper(this)


aozarov · 2016-02-01T22:35:41Z

Looks great. Few minor comments/suggestions.

…tion to TableDefinition

Remove TableInfo hierarchy, add TableType hierarchy

🤖 I have created a release \*beep\* \*boop\* --- ### Updating meta-information for bleeding-edge SNAPSHOT release. --- This PR was generated with [Release Please](https://github.com/googleapis/release-please). See [documentation](https://github.com/googleapis/release-please#release-please).

* chore(bazel): update protobuf to v3.21.7 PiperOrigin-RevId: 477955264 Source-Link: googleapis/googleapis@a724450 Source-Link: googleapis/googleapis-gen@4abcbca Copy-Tag: eyJwIjoiLmdpdGh1Yi8uT3dsQm90LnlhbWwiLCJoIjoiNGFiY2JjYWVjODU1ZTc0YTBiMjJhNDk4OGNmOWUwZWI2MWE4MzA5NCJ9 * 🦉 Updates from OwlBot post-processor See https://github.com/googleapis/repo-automation-bots/blob/main/packages/owl-bot/README.md Co-authored-by: Owl Bot <gcf-owl-bot[bot]@users.noreply.github.com>

* chore: fix auto-release * chore: remove codecov.yml * chore: update license headers for yaml files Source-Link: googleapis/synthtool@5b77727 Post-Processor: gcr.io/cloud-devrel-public-resources/owlbot-java:latest@sha256:ebc2104854c5b81c6fd72ca79400a2e20e0d510c5e0654fd1a19e5c9be160ca6 Co-authored-by: Owl Bot <gcf-owl-bot[bot]@users.noreply.github.com>

Remove TableInfo hierarchy, add TableType hierarchy

c010bc8

mziccard added the api: bigquery Issues related to the BigQuery API. label Jan 29, 2016

googlebot added the cla: yes This human has signed the Contributor License Agreement. label Jan 29, 2016

ajkannan reviewed Jan 29, 2016
View reviewed changes

ajkannan reviewed Jan 30, 2016
View reviewed changes

mziccard force-pushed the bigquery-hierachies branch 2 times, most recently from 3474f3b to 04fbf41 Compare February 1, 2016 14:17

Rename TableType to TableDefinition and other minor fixes

931b4d3

mziccard force-pushed the bigquery-hierachies branch from 04fbf41 to 931b4d3 Compare February 1, 2016 16:21

Fix minor docs and style errors

36d9dc3

aozarov reviewed Feb 1, 2016
View reviewed changes

Rename DefaultTableDefinition to TableDefinition

085903a

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

aozarov reviewed Feb 1, 2016
View reviewed changes

gcloud-java-bigquery/src/main/java/com/google/gcloud/bigquery/TableDefinition.java

@Override

public String toString() {

return MoreObjects.toStringHelper(this)

This comment was marked as spam.

Sign in to view

This comment was marked as spam.

Sign in to view

Rename TableDefinition to StandardTableDefinition and BaseTableDefini…

28a457e

…tion to TableDefinition

aozarov added a commit that referenced this pull request Feb 2, 2016

Merge pull request #600 from mziccard/bigquery-hierachies

2bb03eb

Remove TableInfo hierarchy, add TableType hierarchy

aozarov merged commit 2bb03eb into googleapis:master Feb 2, 2016

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Remove TableInfo hierarchy, add TableType hierarchy #600

Remove TableInfo hierarchy, add TableType hierarchy #600

mziccard commented Jan 29, 2016

This comment was marked as spam.

ajkannan commented Jan 30, 2016

This comment was marked as spam.

ajkannan commented Jan 30, 2016

mziccard commented Feb 1, 2016

ajkannan commented Feb 1, 2016

This comment was marked as spam.

This comment was marked as spam.

mziccard commented Feb 1, 2016

mziccard commented Feb 1, 2016

This comment was marked as spam.

This comment was marked as spam.

This comment was marked as spam.

aozarov commented Feb 1, 2016

Remove TableInfo hierarchy, add TableType hierarchy #600

Remove TableInfo hierarchy, add TableType hierarchy #600

Conversation

mziccard commented Jan 29, 2016

This comment was marked as spam.

ajkannan commented Jan 30, 2016

This comment was marked as spam.

ajkannan commented Jan 30, 2016

mziccard commented Feb 1, 2016

ajkannan commented Feb 1, 2016

This comment was marked as spam.

This comment was marked as spam.

mziccard commented Feb 1, 2016

mziccard commented Feb 1, 2016

This comment was marked as spam.

This comment was marked as spam.

This comment was marked as spam.

aozarov commented Feb 1, 2016