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

Added implementation for Google cloud storage #120

Merged
merged 14 commits into from
Nov 25, 2024
Merged

Added implementation for Google cloud storage #120

merged 14 commits into from
Nov 25, 2024

Conversation

Animagne
Copy link
Collaborator

Implementation for google cloud storage. The biggest gap between this and our other implementations is that metadata setting for objects is not supported in some of the workflows, because it is made in a separate request.


## About this package

This package contains implementations for object storage interfaces exposed by `@itwin/object-storage-core` which allow to consume Azure Blob Storage service. Communication with the Blob Service is implemented using using [Azure Storage Blob client library](https://www.npmjs.com/package/@azure/storage-blob).
Copy link
Collaborator

Choose a reason for hiding this comment

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

Not Azure

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Updated

{
"name": "@itwin/object-storage-google",
"version": "2.2.5",
"description": "Object storage implementation using Azure Blob Storage",
Copy link
Collaborator

Choose a reason for hiding this comment

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

Google

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Updated

FrontendUrlDownloadInput,
} from "@itwin/object-storage-core/lib/frontend";

export const uploadFileSizeLimit = 5_000_000_000; // 5GB
Copy link
Collaborator

Choose a reason for hiding this comment

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

Is this limit set somewhere?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

It's not used. I've removed it and this file and merged the changes into original frontend Helpers file.

@Animagne Animagne merged commit efac07d into main Nov 25, 2024
6 checks passed
@Animagne Animagne deleted the GoogleStorage branch November 25, 2024 08:45
100
)) {
for (const object of objectPage) {
await this.deleteObject(object);

Choose a reason for hiding this comment

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

If an error were to occur during this deletion, is there error handling above to recover?

Also, this waits for each object to be deleted one at a time, it might be more efficient to gather all of the promises into an array and await all simultaneously

Copy link
Collaborator Author

@Animagne Animagne Dec 4, 2024

Choose a reason for hiding this comment

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

If some files failed to be deleted, you could retry it again and delete rest of the files.

We will see how the performance is in real use cases, because right now we don't have any performance metrics. Because this has to conform to the same deleteBaseDirectory method signature as other clients, we can't expose page size and deletion in parallel could be more taxing on the storage.

});
const client = await googleAuth.getClient();
const downscopedClient = new DownscopedClient(client, cab);
const { token, expirationTime } = await downscopedClient.getAccessToken();

Choose a reason for hiding this comment

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

Are there any tests that validate the access tokens returned by this function/the roles that token possesses? Curious to see how they look, I know that google uses its own JWT format

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

We have integration tests shared across all of the packages, for example this describe would test with these tokens. The tests can be ran through VSCode and you can use just default application credentials if you're logged in with your account.

scopes: "https://www.googleapis.com/auth/cloud-platform",
projectId: this._config.projectId,
});
const client = await googleAuth.getClient();

Choose a reason for hiding this comment

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

getClient() uses default access creds, which can be obtained from gcloud, but in a service setting we would need to use env.GOOGLE_APPLICATION_CREDENTIALS and provide a service key correct? Have we decided on the best way to obtain a service account service key in our pipelines?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I don't know if it's necessarily the best long term solution, but right now we just store those credentials as secret files in Azure DevOps.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants