Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

docs(samples): add create range partition table #458

Merged

Conversation

pmakani
Copy link
Contributor

@pmakani pmakani commented Jun 17, 2020

Thank you for opening a Pull Request! Before submitting your PR, there are a few things you can do to make sure it goes smoothly:

  • Make sure to open an issue as a bug/issue before writing your code! That way we can discuss the change, evaluate designs, and agree on the general idea
  • Ensure the tests and linter pass
  • Code coverage does not decrease (if any source code was changed)
  • Appropriate docs were updated (if necessary)

@pmakani pmakani requested a review from stephaniewang526 Jun 17, 2020
@googlebot googlebot added the cla: yes label Jun 17, 2020
@codecov
Copy link

@codecov codecov bot commented Jun 17, 2020

Codecov Report

Merging #458 into master will not change coverage.
The diff coverage is n/a.

Impacted file tree graph

@@            Coverage Diff            @@
##             master     #458   +/-   ##
=========================================
  Coverage     81.36%   81.36%           
  Complexity     1225     1225           
=========================================
  Files            77       77           
  Lines          6218     6218           
  Branches        690      691    +1     
=========================================
  Hits           5059     5059           
  Misses          802      802           
  Partials        357      357           

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update d16a3b5...747dbea. Read the comment docs.

import com.google.cloud.bigquery.TableId;
import com.google.cloud.bigquery.TableInfo;

// [START bigquery_create_table_range_partitioned]
Copy link
Member

@stephaniewang526 stephaniewang526 Jun 17, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this needs to go above import statements so that ppl know what are needed

private ByteArrayOutputStream bout;
private PrintStream out;

private static final String BIGQUERY_DATASET_NAME = System.getenv("BIGQUERY_DATASET_NAME");
Copy link
Member

@stephaniewang526 stephaniewang526 Jun 17, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We've decided to just call requireEnv method here to eliminate one other method called. please see this sample for details -

private static final String BIGQUERY_DATASET_NAME = requireEnvVar("BIGQUERY_DATASET_NAME");

assertThat(bout.toString()).contains("Range partitioned table created successfully");

// Clean up
DeleteTable.deleteTable(BIGQUERY_DATASET_NAME, tableName);
Copy link
Member

@stephaniewang526 stephaniewang526 Jun 17, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

let's put this in teardown() - see sample

DeleteTable.deleteTable(BIGQUERY_DATASET_NAME, tableName);

@pmakani pmakani added kokoro:force-run kokoro:run and removed kokoro:force-run kokoro:run labels Jun 17, 2020
@yoshi-kokoro yoshi-kokoro removed the kokoro:force-run label Jun 17, 2020
@pmakani pmakani added the kokoro:force-run label Jun 17, 2020
@yoshi-kokoro yoshi-kokoro removed the kokoro:force-run label Jun 17, 2020
@stephaniewang526
Copy link
Member

@stephaniewang526 stephaniewang526 commented Jun 17, 2020

	at org.junit.Assert.fail(Assert.java:89)
	at org.junit.Assert.failNotEquals(Assert.java:835)
	at org.junit.Assert.assertEquals(Assert.java:647)
	at org.junit.Assert.assertEquals(Assert.java:633)
	at com.google.cloud.bigquery.it.ITBigQueryTest.testListPartitions(ITBigQueryTest.java:849)
	at sun.reflect.NativeMethodAccessorImpl.invoke0(Native Method)
	at sun.reflect.NativeMethodAccessorImpl.invoke(NativeMethodAccessorImpl.java:62)
	at sun.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:43)
	at java.lang.reflect.Method.invoke(Method.java:498)
	at org.junit.runners.model.FrameworkMethod$1.runReflectiveCall(FrameworkMethod.java:59)
	at org.junit.internal.runners.model.ReflectiveCallable.run(ReflectiveCallable.java:12)
	at org.junit.runners.model.FrameworkMethod.invokeExplosively(FrameworkMethod.java:56)
	at org.junit.internal.runners.statements.InvokeMethod.evaluate(InvokeMethod.java:17)
	at org.junit.internal.runners.statements.FailOnTimeout$CallableStatement.call(FailOnTimeout.java:288)
	at org.junit.internal.runners.statements.FailOnTimeout$CallableStatement.call(FailOnTimeout.java:282)
	at java.util.concurrent.FutureTask.run(FutureTask.java:266)
	at java.lang.Thread.run(Thread.java:748)```

seeing this IT failure

@stephaniewang526 stephaniewang526 merged commit 68140d5 into googleapis:master Jun 17, 2020
15 checks passed
@pmakani pmakani deleted the create-range-partition-table branch Jun 18, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
cla: yes
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants