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: Added Google Cloud Storage support for Import Records. #15362

Closed
wants to merge 2 commits into from

Conversation

Jessedev1
Copy link
Contributor

Description

When using an external file system like Google Cloud Storage, a stream must be opened to access the file; otherwise, it cannot be read. While the implementation for S3 already existed, support for GCS was missing. Therefore, I implemented it.

Visual changes

This is not applicable.

Functional changes

  • Code style has been fixed by running the composer cs command.
  • Changes have been tested to not break existing functionality.
  • Documentation is up-to-date.

@danharrin
Copy link
Member

As a rule, we do not support GCS anywhere else in Filament. By merging this PR, it feels like I am then committing to providing full support for it elsewhere, which makes me very hesitant. I have no experience with it and it seems to differ from S3 quite a lot. Since I have not had this request before, I am willing to bet there are a relatively low number of users who are using GCS with Filament.

As such, I am not going to merge this. Maybe if GCS gains more popularity in the future, I will choose to introduce official support everywhere. For now, you could extend the ImportAction in your own application to add GCS support, or even release a package which does so. Sorry. Please check out our contributing guide which talks about proposing the feature before submitting the PR, to ensure you aren't spending time building something that we won't merge.

@danharrin danharrin closed this Jan 22, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Archived in project
Development

Successfully merging this pull request may close these issues.

2 participants