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): create and query Amazon s3 data using external table #835

Conversation

stephaniewang526
Copy link
Member

@stephaniewang526 stephaniewang526 commented Oct 20, 2020

No description provided.

@stephaniewang526 stephaniewang526 requested review from and pmakani Oct 20, 2020
@google-cla
Copy link

@google-cla google-cla bot commented Oct 20, 2020

All (the pull request submitter and all commit authors) CLAs are signed, but one or more commits were authored or co-authored by someone other than the pull request submitter.

We need to confirm that all authors are ok with their commits being contributed to this project. Please have them confirm that by leaving a comment that contains only @googlebot I consent. in this pull request.

Note to project maintainer: There may be cases where the author cannot leave a comment, or the comment is not properly detected as consent. In those cases, you can manually confirm consent of the commit author(s), and set the cla label to yes (if enabled on your project).

ℹ️ Googlers: Go here for more info.

@google-cla google-cla bot added the cla: no label Oct 20, 2020
@stephaniewang526 stephaniewang526 added cla: yes and removed cla: no labels Oct 20, 2020
@google-cla
Copy link

@google-cla google-cla bot commented Oct 20, 2020

All (the pull request submitter and all commit authors) CLAs are signed, but one or more commits were authored or co-authored by someone other than the pull request submitter.

We need to confirm that all authors are ok with their commits being contributed to this project. Please have them confirm that by leaving a comment that contains only @googlebot I consent. in this pull request.

Note to project maintainer: There may be cases where the author cannot leave a comment, or the comment is not properly detected as consent. In those cases, you can manually confirm consent of the commit author(s), and set the cla label to yes (if enabled on your project).

ℹ️ Googlers: Go here for more info.

@google-cla google-cla bot added cla: no and removed cla: yes labels Oct 20, 2020
@stephaniewang526 stephaniewang526 added cla: yes kokoro:force-run and removed cla: no labels Oct 20, 2020
@yoshi-kokoro yoshi-kokoro removed the kokoro:force-run label Oct 20, 2020
@codecov
Copy link

@codecov codecov bot commented Oct 20, 2020

Codecov Report

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

Impacted file tree graph

@@            Coverage Diff            @@
##             master     #835   +/-   ##
=========================================
  Coverage     80.37%   80.37%           
  Complexity     1268     1268           
=========================================
  Files            79       79           
  Lines          6548     6548           
  Branches        749      749           
=========================================
  Hits           5263     5263           
  Misses          892      892           
  Partials        393      393           

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 afe3b18...01bccfe. Read the comment docs.

lesv
lesv approved these changes Oct 20, 2020
Copy link

@lesv lesv left a comment

Consider the questions

} catch (BigQueryException | InterruptedException e) {
System.out.println("Query not performed \n" + e.toString());
}
Copy link

@lesv lesv Oct 20, 2020

Choose a reason for hiding this comment

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

Should these be caught? Perhaps just mention them or let them propagate? NOTE - BigQueryException is a RuntimeException and InterruptedException is not.

Copy link

@lesv lesv Oct 20, 2020

Choose a reason for hiding this comment

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

More important is why might these have been thrown and what should a user really do with them?

Copy link
Member Author

@stephaniewang526 stephaniewang526 Oct 20, 2020

Choose a reason for hiding this comment

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

I think we will catch BigQueryException and not InterruptedException.
Updated!

@@ -120,18 +90,19 @@ public void tearDown() throws IOException {

@Test
public void testCreateExternalTableAws() {
String sourceUri = "s3://cloud-samples-tests/us-states.csv";
String sourceUri = "s3://steph-omni-test-bucket/us-states.csv";
Copy link

@lesv lesv Oct 20, 2020

Choose a reason for hiding this comment

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

Might not be your best final choice for a bucket name.

Copy link
Member Author

@stephaniewang526 stephaniewang526 Oct 20, 2020

Choose a reason for hiding this comment

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

Yes - you're right. Created a new bucket for devrel sampels testing. Thank you!

@google-cla
Copy link

@google-cla google-cla bot commented Oct 20, 2020

All (the pull request submitter and all commit authors) CLAs are signed, but one or more commits were authored or co-authored by someone other than the pull request submitter.

We need to confirm that all authors are ok with their commits being contributed to this project. Please have them confirm that by leaving a comment that contains only @googlebot I consent. in this pull request.

Note to project maintainer: There may be cases where the author cannot leave a comment, or the comment is not properly detected as consent. In those cases, you can manually confirm consent of the commit author(s), and set the cla label to yes (if enabled on your project).

ℹ️ Googlers: Go here for more info.

@google-cla google-cla bot added cla: no and removed cla: yes labels Oct 20, 2020
@stephaniewang526 stephaniewang526 removed the cla: no label Oct 20, 2020
@stephaniewang526 stephaniewang526 added the cla: yes label Oct 20, 2020
@stephaniewang526 stephaniewang526 merged commit 53a56be into googleapis:master Oct 21, 2020
21 checks passed
gcf-merge-on-green bot pushed a commit that referenced this issue Oct 23, 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

3 participants