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

Fix blob creation in trino file system tests #19678

Closed
wants to merge 1 commit into from

Conversation

elonazoulay
Copy link
Member

Description

Additional context and related issues

Release notes

( ) This is not user-visible or is docs only, and no release notes are required.
( ) 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 Nov 8, 2023
@elonazoulay
Copy link
Member Author

Note: if the root location is not empty then testDirectoryExists was double appending the path when calling createBlob - lmk if this fix is ok.

@elonazoulay
Copy link
Member Author

I can also add the RemoteStorageHelper back, I will need the project id used for ci tests - either way createBlob needs to be changed (comment above).

String bucket = RemoteStorageHelper.generateBucketName();
storage.create(BucketInfo.of(bucket));
this.rootLocation = Location.of("gs://%s/".formatted(bucket));
if (storage.get(bucket) == null) {
Copy link
Member Author

@elonazoulay elonazoulay Nov 8, 2023

Choose a reason for hiding this comment

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

Should the bucket be created if it does not exist?

@elonazoulay elonazoulay force-pushed the fix_ci_gcs_tests branch 3 times, most recently from 41b012e to 08595cc Compare November 9, 2023 01:33
@electrum
Copy link
Member

electrum commented Nov 9, 2023

/test-with-secrets sha=08595ccc84d539eadba1fe012114f25181b79e6c

Copy link

github-actions bot commented Nov 9, 2023

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

Copy link
Member

@electrum electrum left a comment

Choose a reason for hiding this comment

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

We should go with the original approach of creating the bucket in the test, since we want to test that the code works with the root of the bucket. This is what we do for S3 and Azure.

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.

2 participants