Skip to content

BigQuery FTE#15620

Merged
losipiuk merged 2 commits intotrinodb:masterfrom
mwd410:mdeady-bigquery-fte-3877
Jan 20, 2023
Merged

BigQuery FTE#15620
losipiuk merged 2 commits intotrinodb:masterfrom
mwd410:mdeady-bigquery-fte-3877

Conversation

@mwd410
Copy link
Copy Markdown
Contributor

@mwd410 mwd410 commented Jan 5, 2023

Description

This PR introduces FTE for BigQuery.

This differs slightly from previous FTE PRs in that the FailureRecoveryTests are executed within the -Pcloud-tests, rather than a separate profile like the others. This may cause problems by running up against the 1 hour time limit, in which case, we'll have to separate the FailureRecoveryTests from cloud-tests.

Additional context and related issues

Release notes

( ) This is not user-visible or docs only and no release notes are required.
(X) Release notes are required, please propose a release note for me.
( ) Release notes are required, with the following suggested text:

# Section
* Fix some things. ({issue}`issuenumber`)

@cla-bot cla-bot bot added the cla-signed label Jan 5, 2023
@losipiuk
Copy link
Copy Markdown
Member

losipiuk commented Jan 5, 2023

/test-with-secrets a0db7441684de62e56d8e9a98a6d7428912fd1d7

@github-actions github-actions bot added the docs label Jan 5, 2023
@arhimondr
Copy link
Copy Markdown
Contributor

/test-with-secrets sha=a0db7441684de62e56d8e9a98a6d7428912fd1d7

1 similar comment
@losipiuk
Copy link
Copy Markdown
Member

losipiuk commented Jan 6, 2023

/test-with-secrets sha=a0db7441684de62e56d8e9a98a6d7428912fd1d7

@nineinchnick
Copy link
Copy Markdown
Member

Here's the latest workflow run with secrets: https://github.com/trinodb/trino/actions/runs/3857770553

@github-actions
Copy link
Copy Markdown

github-actions bot commented Jan 6, 2023

The CI workflow run with tests that require additional secrets finished as failure: https://github.com/trinodb/trino/actions/runs/3857770553

@mwd410 mwd410 force-pushed the mdeady-bigquery-fte-3877 branch from a0db744 to ae199d1 Compare January 9, 2023 18:45
@losipiuk
Copy link
Copy Markdown
Member

losipiuk commented Jan 9, 2023

/test-with-secrets sha=ae199d10b18460e09a7989b2e1e4097b274a1982

Copy link
Copy Markdown
Member

@ebyhr ebyhr left a comment

Choose a reason for hiding this comment

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

I would leave the final review & merge to @arhimondr @losipiuk

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

nit: QueryJobConfiguration.of accepts a sql and build the object.

Comment on lines 539 to 544
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

schemaName is already remote schema name. Calling getRemoteSchemaName method is redundant.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Suggested change
private TableId createTable(BigQueryClient client, String projectId, String remoteSchemaName, String tableName, ImmutableList<Field> fields, Optional<String> tableComment)
private TableId createTable(BigQueryClient client, String projectId, String datasetName, String tableName, List<Field> fields, Optional<String> tableComment)

Comment on lines 538 to 539
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

BigQueryTableHandle#asPlainTable returns BigQueryNamedRelationHandle.

@mwd410 mwd410 force-pushed the mdeady-bigquery-fte-3877 branch 2 times, most recently from d3fe6c8 to 6fa0767 Compare January 13, 2023 15:06
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

nit: quote above vs quoted here looks kinda unnatural. I would keep name the same.

@losipiuk
Copy link
Copy Markdown
Member

/test-with-secrets sha=6c5eb9f63905c55b99fc49e561c1cc95579063f6

@losipiuk
Copy link
Copy Markdown
Member

/test-with-secrets sha=6fa0767cc2d96008f2876bfa307693c4a267d79f

@losipiuk
Copy link
Copy Markdown
Member

/test-with-secrets sha=6fa0767cc2d96008f2876bfa307693c4a267d79f

run in progress:https://github.com/trinodb/trino/actions/runs/3931242353

@github-actions
Copy link
Copy Markdown

The CI workflow run with tests that require additional secrets finished as failure: https://github.com/trinodb/trino/actions/runs/3931242353

@mwd410 mwd410 force-pushed the mdeady-bigquery-fte-3877 branch from 6fa0767 to d8a1e6e Compare January 17, 2023 21:33
@losipiuk
Copy link
Copy Markdown
Member

/test-with-secrets sha=d8a1e6e3a29aac958bd95ec5ce3f950d7e7c1cc4

@mwd410 mwd410 force-pushed the mdeady-bigquery-fte-3877 branch from d8a1e6e to 62b32d8 Compare January 19, 2023 16:43
@losipiuk
Copy link
Copy Markdown
Member

/test-with-secrets sha= 62b32d8cce2db86e290339d2c851b90985389009

@losipiuk
Copy link
Copy Markdown
Member

/test-with-secrets sha=62b32d8cce2db86e290339d2c851b90985389009

@github-actions
Copy link
Copy Markdown

The CI workflow run with tests that require additional secrets finished as failure: https://github.com/trinodb/trino/actions/runs/3960804172

@losipiuk losipiuk force-pushed the mdeady-bigquery-fte-3877 branch from 62b32d8 to d53a558 Compare January 20, 2023 10:44
@losipiuk
Copy link
Copy Markdown
Member

/test-with-secrets sha=d53a55847e0d026a9589374e773b5770ddc8a12a

@losipiuk
Copy link
Copy Markdown
Member

cancelled local CI as I am running version "with secrets". And we care about output of the latter.

@losipiuk
Copy link
Copy Markdown
Member

@github-actions
Copy link
Copy Markdown

The CI workflow run with tests that require additional secrets finished as failure: https://github.com/trinodb/trino/actions/runs/3967428518

@losipiuk
Copy link
Copy Markdown
Member

/test-with-secrets sha=d53a55847e0d026a9589374e773b5770ddc8a12a

@losipiuk
Copy link
Copy Markdown
Member

I got #15801 but it looks totally unrelated (it happens in different test job). merging.

@losipiuk losipiuk merged commit ce7d3f7 into trinodb:master Jan 20, 2023
@mwd410 mwd410 deleted the mdeady-bigquery-fte-3877 branch January 20, 2023 17:52
@github-actions github-actions bot added this to the 406 milestone Jan 20, 2023
@github-actions
Copy link
Copy Markdown

The CI workflow run with tests that require additional secrets finished as failure: https://github.com/trinodb/trino/actions/runs/3969882477

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

Development

Successfully merging this pull request may close these issues.

5 participants