Skip to content

feat(gcs): allow unauthenticated requests#4965

Merged
Xuanwo merged 11 commits intoapache:mainfrom
jdockerty:feat/gcs-allow-no-auth
Aug 7, 2024
Merged

feat(gcs): allow unauthenticated requests#4965
Xuanwo merged 11 commits intoapache:mainfrom
jdockerty:feat/gcs-allow-no-auth

Conversation

@jdockerty
Copy link
Copy Markdown
Contributor

@jdockerty jdockerty commented Aug 5, 2024 •
edited
Loading

Which issue does this PR close?

Closes #4963

What changes are included in this PR?

Introduction of:

  • allow_anonymous
  • disable_vm_metadata
  • disable_config_load

to GcsConfig and their corresponding plumbing within GcsCore.

Are there any user-facing changes?

Changes are internal only.

@github-actions github-actions Bot added the releases-note/feat The PR implements a new feature or has a title that begins with "feat" label Aug 5, 2024
jdockerty
jdockerty commented Aug 5, 2024
View reviewed changes
Comment thread core/src/services/gcs/core.rs
Xuanwo
Xuanwo reviewed Aug 6, 2024
View reviewed changes
Comment thread core/src/services/gcs/backend.rs Outdated
Comment thread core/src/services/gcs/backend.rs Outdated
@Xuanwo Xuanwo mentioned this pull request Aug 6, 2024
Closed
@jdockerty jdockerty marked this pull request as ready for review August 6, 2024 13:30
@jdockerty jdockerty mentioned this pull request Aug 7, 2024
Merged
@jdockerty
Copy link
Copy Markdown
Contributor Author

jdockerty commented Aug 7, 2024 •
edited
Loading

@Xuanwo Using this version of opendal locally for my iceberg-rust changes works with the DockerCompose setup. I believe this should be ready to take a look at.

Is there anything else you'd like me to include here?

I see there's the use of an AtomicBool for tracking previously loaded credentials within the S3 core, is that relevant to also do with this GCS change or better for another PR?

@Xuanwo
Copy link
Copy Markdown
Member

Xuanwo commented Aug 7, 2024

I see there's the use of an AtomicBool for tracking previously loaded credentials within the S3 core, is that relevant to also do with this GCS change or better for another PR?

That's fine to include in another PR.

Xuanwo
Xuanwo reviewed Aug 7, 2024
View reviewed changes
Copy link
Copy Markdown
Member

@Xuanwo Xuanwo left a comment •
edited
Loading

Choose a reason for hiding this comment

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

Looks mostly good to me, just needs some semantic clarity.

Comment thread core/src/services/gcs/backend.rs Outdated
Comment thread core/src/services/gcs/core.rs Outdated
Comment thread core/src/services/gcs/backend.rs Outdated
Xuanwo
Xuanwo approved these changes Aug 7, 2024
View reviewed changes
Copy link
Copy Markdown
Member

@Xuanwo Xuanwo left a comment

Choose a reason for hiding this comment

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

Excellent PR and great communication! Happy to work with you.

@Xuanwo Xuanwo merged commit 0a3e98f into apache:main Aug 7, 2024
@Xuanwo
Copy link
Copy Markdown
Member

Xuanwo commented Aug 7, 2024

Perhaps you might be interested in assisting with #4876 😆

@jdockerty jdockerty deleted the feat/gcs-allow-no-auth branch August 7, 2024 14:24
@jdockerty jdockerty mentioned this pull request Aug 7, 2024
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@Xuanwo Xuanwo Xuanwo approved these changes

Assignees

No one assigned

Labels

releases-note/feat The PR implements a new feature or has a title that begins with "feat"

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

new feature: allow GCP unauthenticated requests

2 participants

@jdockerty @Xuanwo