Skip to content

[FLINK-39331] Add @Experimental annotation to public Native S3 classes#27833

Open
Samrat002 wants to merge 1 commit intoapache:masterfrom
Samrat002:Add-experimental-annotation
Open

[FLINK-39331] Add @Experimental annotation to public Native S3 classes#27833
Samrat002 wants to merge 1 commit intoapache:masterfrom
Samrat002:Add-experimental-annotation

Conversation

@Samrat002
Copy link
Copy Markdown
Contributor

@Samrat002 Samrat002 commented Mar 26, 2026

What is the purpose of the change

Mark the public API classes in flink-s3-fs-native as experimental to signal to users that this feature is still evolving and may undergo changes in future releases.

Brief change log

Mark following public class with @Experimental

  1. NativeS3FileSystemFactory
  2. NativeS3AFileSystemFactory
  3. NativeS3OutputStream
  4. NativeS3RecoverableFsDataOutputStream
  5. NativeS3Committer

Verifying this change

Existing UT suffices.

Does this pull request potentially affect one of the following parts:

  • Dependencies: no
  • The public API, i.e., is any changed class annotated with @Public(Evolving): yes (marked as @Experimental)
  • The serializers: no
  • The runtime per-record code paths (performance sensitive): no
  • Anything that affects deployment or recovery: no
  • The S3 file system connector: yes

Documentation

  • Does this pull request introduce a new feature? no
  • If yes, how is the feature documented? not applicable

@flinkbot
Copy link
Copy Markdown
Collaborator

flinkbot commented Mar 26, 2026 •
edited
Loading

CI report:

Bot commands The @flinkbot bot supports the following commands:
  • @flinkbot run azure re-run the last Azure build

Izeren
Izeren reviewed Mar 26, 2026
View reviewed changes
Copy link
Copy Markdown
Contributor

@Izeren Izeren left a comment

Choose a reason for hiding this comment

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

Thank you for the PR @Samrat002, I wonder if we should rather reduce visibility of the classes than adding experimental annotation to them (except for factories)

@Izeren
Copy link
Copy Markdown
Contributor

Izeren commented Mar 26, 2026 •
edited
Loading

@Samrat002, In addition, should we downgrade: NativeS3FileSystem, NativeS3InputStream, NativeS3Recoverable, NativeS3RecoverableSerializer to package-private or Internal@?

Also, NativeS3RecoverableWriter is marked as @PublicEvolving, and it should be Experimental@ as well

@github-actions github-actions bot added the community-reviewed PR has been reviewed by the community. label Mar 26, 2026
@Samrat002
Copy link
Copy Markdown
Contributor Author

Samrat002 commented Mar 27, 2026

@Samrat002, In addition, should we downgrade: NativeS3FileSystem, NativeS3InputStream, NativeS3Recoverable, NativeS3RecoverableSerializer to package-private or Internal@?

Yes . right thing to do and reduce the API visiblity . i have made the classes NativeS3FileSystem, NativeS3InputStream, NativeS3Recoverable, NativeS3RecoverableSerializer to package-private

Also, NativeS3RecoverableWriter is marked as @PublicEvolving, and it should be Experimental@ as well

yes. my miss. fixed it now

@Samrat002 Samrat002 requested a review from Izeren March 27, 2026 06:45
Izeren
Izeren suggested changes Mar 27, 2026
View reviewed changes
Copy link
Copy Markdown
Contributor

@Izeren Izeren left a comment

Choose a reason for hiding this comment

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

NativeS3Recoverable, NativeS3RecoverableSerializer seems to be still public

@Samrat002 Samrat002 force-pushed the Add-experimental-annotation branch from 1f8d3e7 to 9eda347 Compare March 27, 2026 16:51
@Samrat002 Samrat002 requested a review from Izeren March 27, 2026 16:52
Izeren
Izeren approved these changes Mar 27, 2026
View reviewed changes
Copy link
Copy Markdown
Contributor

@Izeren Izeren left a comment

Choose a reason for hiding this comment

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

LGTM, approved with nit, assuming that the tests are green, thank you for the change @Samrat002

@Samrat002
[FLINK-39331] Add @experimental annotation to public S3 classes
3f07940 
Make NativeS3Recoverable and NativeS3RecoverableSerializer package private
@Samrat002 Samrat002 force-pushed the Add-experimental-annotation branch from 9eda347 to 3f07940 Compare March 28, 2026 03:08
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

1 more reviewer

@Izeren Izeren Izeren approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

community-reviewed PR has been reviewed by the community.

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@Samrat002 @flinkbot @Izeren