Skip to content

Comments

[improve] Adjust CosFile connector instantiation#9121

Closed
misi1987107 wants to merge 8 commits intoapache:devfrom
misi1987107:feature-file-cos-connector-creation-modify
Closed

[improve] Adjust CosFile connector instantiation#9121
misi1987107 wants to merge 8 commits intoapache:devfrom
misi1987107:feature-file-cos-connector-creation-modify

Conversation

@misi1987107
Copy link
Contributor

Purpose of this pull request

subtask of #8576

Adjust CosFile connector instantiation

Does this PR introduce any user-facing change?

How was this patch tested?

Check list

Comment on lines 78 to 82
@Override
public void setTypeInfo(SeaTunnelRowType seaTunnelRowType) {
this.seaTunnelRowType = seaTunnelRowType;
this.fileSinkConfig = new FileSinkConfig(pluginConfig, seaTunnelRowType);
}
Copy link
Member

Choose a reason for hiding this comment

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

Why delete this? We still have some sink should invoke it like OssFileSink, HdfsFileSink etc.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Sorry,I neet to make adjustments according to purpose2 ,move the connector creation logic from Connector to ConnectorFactory.I don't know how to adjust it
(#8576 (comment))

Comment on lines 71 to 74
@Override
public List<CatalogTable> getProducedCatalogTables() {
return SeaTunnelSource.super.getProducedCatalogTables();
}
Copy link
Member

Choose a reason for hiding this comment

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

We should return catalog table in here to replace getProducedType method.

(SeaTunnelSource<T, SplitT, StateT>)
new CosFileSource(
context.getOptions(),
CatalogTableUtil.buildWithConfig(context.getOptions()));
Copy link
Member

Choose a reason for hiding this comment

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

@misi1987107
Copy link
Contributor Author

misi1987107 commented Apr 10, 2025

#8576 (comment)
About purpose2,can i adjust it like this?Please give me some advice. @Hisoka-X @liunaijie

@github-actions github-actions bot added the core SeaTunnel core module label Apr 14, 2025
@misi1987107
Copy link
Contributor Author

I used the factory class to create CosFileSource and CosFileLink connectors, but there is a check method testAllConnectorImplementeFactoryWithUpToDateMethod that does not allow deprecated methods, so I skipped the check.
Please refer
#9121 (comment)
#9121 (comment)

@Hisoka-X
Copy link
Member

I used the factory class to create CosFileSource and CosFileLink connectors, but there is a check method testAllConnectorImplementeFactoryWithUpToDateMethod that does not allow deprecated methods, so I skipped the check.

Please do not do that. The sub reason of #8576 is make sure all method up to date without deprecated method.

@misi1987107
Copy link
Contributor Author

misi1987107 commented Apr 16, 2025

I used the factory class to create CosFileSource and CosFileLink connectors, but there is a check method testAllConnectorImplementeFactoryWithUpToDateMethod that does not allow deprecated methods, so I skipped the check.

Please do not do that. The sub reason of #8576 is make sure all method up to date without deprecated method.

If I don't skip the check method testAllConnectorImplementFactoryWithUpToDateMethod(),I must delete the method BaseFileSink.setTypeInfo(),otherwise, it cannot pass through.
image

@nielifeng nielifeng requested a review from Copilot April 16, 2025 06:55
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Copilot reviewed 5 out of 5 changed files in this pull request and generated 2 comments.

Comment on lines +76 to +77
// adjusted the connector implementation,not deal with deprecated method yet
blockList.add("CosFileSourceFactory");
Copy link

Copilot AI Apr 16, 2025

Choose a reason for hiding this comment

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

[nitpick] Consider adding a space after the comma for clarity, e.g., 'connector implementation, not dealing with deprecated method yet'.

Suggested change
// adjusted the connector implementation,not deal with deprecated method yet
blockList.add("CosFileSourceFactory");
// adjusted the connector implementation, not deal with deprecated method yet

Copilot uses AI. Check for mistakes.
@Override
public <T, SplitT extends SourceSplit, StateT extends Serializable>
TableSource<T, SplitT, StateT> createSource(TableSourceFactoryContext context) {
return () -> (SeaTunnelSource<T, SplitT, StateT>) new CosFileSource(context.getOptions());
Copy link

Copilot AI Apr 16, 2025

Choose a reason for hiding this comment

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

Consider adding an annotation to suppress the unchecked cast warning or handling the cast more explicitly to ensure type safety.

Suggested change
return () -> (SeaTunnelSource<T, SplitT, StateT>) new CosFileSource(context.getOptions());
SeaTunnelSource<?, ?, ?> source = new CosFileSource(context.getOptions());
if (!(source instanceof SeaTunnelSource<T, SplitT, StateT>)) {
throw new ClassCastException("The created source is not compatible with the expected type.");
}
return () -> (SeaTunnelSource<T, SplitT, StateT>) source;

Copilot uses AI. Check for mistakes.
@misi1987107 misi1987107 deleted the feature-file-cos-connector-creation-modify branch June 6, 2025 06:54
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

connectors-v2 core SeaTunnel core module file

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants