You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@flink.apache.org by "kaygorodov (via GitHub)" <gi...@apache.org> on 2024/11/18 18:38:41 UTC
[PR] [FLINK-36745][docs] Make FixedSizeSplitFetcherManager and FixedFetcherSizeSourceReader examples match Flink 2.0 API [flink]
kaygorodov opened a new pull request, #25667:
URL: https://github.com/apache/flink/pull/25667
## What is the purpose of the change
Examples on documentation page [Data Sources](https://nightlies.apache.org/flink/flink-docs-master/docs/dev/datastream/sources/) have been using the APIs that do not exist anymore in Flink 2.0.
- FixedSizeSplitFetcherManager uses a SplitFetcherManager constructor that accepts elements queue and was removed in Flink 1.20-preview https://issues.apache.org/jira/browse/FLINK-36245
- FixedFetcherSizeSourceReader uses uses a SourceReaderBase constructor that accepts elements queue and was removed in Flink 1.20-preview https://issues.apache.org/jira/browse/FLINK-36245
- FixedFetcherSizeSourceReader uses getInteger method of Configuration that was removed in in Flink 1.20-preview https://issues.apache.org/jira/browse/FLINK-34082
## Brief change log
- switch FixedSizeSplitFetcherManager, FixedFetcherSizeSourceReader to use proper constructors of their super classes
- switch from getInteger to get method of the Configuration class
## Verifying this change
- Since the changes affect the doc examples and there are no tests. I copied the snippets and verified (compiled) them locally.
## Does this pull request potentially affect one of the following parts:
- Dependencies (does it add or upgrade a dependency): no
- The public API, i.e., is any changed class annotated with `@Public(Evolving)`: no
- The serializers: no
- The runtime per-record code paths (performance sensitive): no
- Anything that affects deployment or recovery: JobManager (and its components), Checkpointing, Kubernetes/Yarn, ZooKeeper: no
- The S3 file system connector: no
## Documentation
- Does this pull request introduce a new feature? no
- If yes, how is the feature documented? not applicable
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@flink.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [FLINK-36745][docs] Make FixedSizeSplitFetcherManager and FixedFetcherSizeSourceReader examples match Flink 2.0-preview API [flink]
Posted by "lvyanquan (via GitHub)" <gi...@apache.org>.
lvyanquan commented on code in PR #25667:
URL: https://github.com/apache/flink/pull/25667#discussion_r1850103994
##########
docs/content/docs/dev/datastream/sources.md:
##########
@@ -325,16 +325,13 @@ public class FixedFetcherSizeSourceReader<E, T, SplitT extends SourceSplit, Spli
extends SourceReaderBase<E, T, SplitT, SplitStateT> {
public FixedFetcherSizeSourceReader(
- FutureCompletingBlockingQueue<RecordsWithSplitIds<E>> elementsQueue,
Supplier<SplitReader<E, SplitT>> splitFetcherSupplier,
RecordEmitter<E, T, SplitStateT> recordEmitter,
Configuration config,
SourceReaderContext context) {
super(
- elementsQueue,
new FixedSizeSplitFetcherManager<>(
- config.getInteger(SourceConfig.NUM_FETCHERS),
- elementsQueue,
+ config.get(SourceConfig.NUM_FETCHERS),
splitFetcherSupplier),
Review Comment:
The construction method of FixedSizeSplitFetcherManager is
```
public FixedSizeSplitFetcherManager(
int numFetchers,
Supplier<SplitReader<E, SplitT>> splitReaderSupplier,
Configuration config)
```
but here you created it with
```
new FixedSizeSplitFetcherManager<>(
config.get(SourceConfig.NUM_FETCHERS),
splitFetcherSupplier)
```
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@flink.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [FLINK-36745][docs] Make FixedSizeSplitFetcherManager and FixedFetcherSizeSourceReader examples match Flink 2.0-preview API [flink]
Posted by "davidradl (via GitHub)" <gi...@apache.org>.
davidradl commented on code in PR #25667:
URL: https://github.com/apache/flink/pull/25667#discussion_r1850415111
##########
docs/content.zh/docs/dev/datastream/sources.md:
##########
@@ -294,9 +294,9 @@ public class FixedSizeSplitFetcherManager<E, SplitT extends SourceSplit>
public FixedSizeSplitFetcherManager(
int numFetchers,
- FutureCompletingBlockingQueue<RecordsWithSplitIds<E>> elementsQueue,
- Supplier<SplitReader<E, SplitT>> splitReaderSupplier) {
- super(elementsQueue, splitReaderSupplier);
+ Supplier<SplitReader<E, SplitT>> splitReaderSupplier,
+ Configuration config) {
Review Comment:
the Chinese version contains : `SourceReaderContext context) {` as the last parameter. Which one is correct?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@flink.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [FLINK-36745][docs] Make FixedSizeSplitFetcherManager and FixedFetcherSizeSourceReader examples match Flink 2.0-preview API [flink]
Posted by "kaygorodov (via GitHub)" <gi...@apache.org>.
kaygorodov commented on code in PR #25667:
URL: https://github.com/apache/flink/pull/25667#discussion_r1849982828
##########
docs/content/docs/dev/datastream/sources.md:
##########
@@ -325,16 +325,13 @@ public class FixedFetcherSizeSourceReader<E, T, SplitT extends SourceSplit, Spli
extends SourceReaderBase<E, T, SplitT, SplitStateT> {
public FixedFetcherSizeSourceReader(
- FutureCompletingBlockingQueue<RecordsWithSplitIds<E>> elementsQueue,
Supplier<SplitReader<E, SplitT>> splitFetcherSupplier,
RecordEmitter<E, T, SplitStateT> recordEmitter,
Configuration config,
SourceReaderContext context) {
super(
- elementsQueue,
new FixedSizeSplitFetcherManager<>(
- config.getInteger(SourceConfig.NUM_FETCHERS),
- elementsQueue,
+ config.get(SourceConfig.NUM_FETCHERS),
splitFetcherSupplier),
Review Comment:
Maybe, I am missing something. It was actually passed :
```java
new FixedSizeSplitFetcherManager<>(
config.getInteger(SourceConfig.NUM_FETCHERS),
```
what I did is changed the removed _getInteger_ to _get_. This code compiles now.
FixedSizeSplitFetcherManager expects an int as the first parameter in its constructor:
```java
public FixedSizeSplitFetcherManager(
int numFetchers,
```
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@flink.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [FLINK-36745][docs] Make FixedSizeSplitFetcherManager and FixedFetcherSizeSourceReader examples match Flink 2.0-preview API [flink]
Posted by "kaygorodov (via GitHub)" <gi...@apache.org>.
kaygorodov commented on code in PR #25667:
URL: https://github.com/apache/flink/pull/25667#discussion_r1850149045
##########
docs/content/docs/dev/datastream/sources.md:
##########
@@ -325,16 +325,13 @@ public class FixedFetcherSizeSourceReader<E, T, SplitT extends SourceSplit, Spli
extends SourceReaderBase<E, T, SplitT, SplitStateT> {
public FixedFetcherSizeSourceReader(
- FutureCompletingBlockingQueue<RecordsWithSplitIds<E>> elementsQueue,
Supplier<SplitReader<E, SplitT>> splitFetcherSupplier,
RecordEmitter<E, T, SplitStateT> recordEmitter,
Configuration config,
SourceReaderContext context) {
super(
- elementsQueue,
new FixedSizeSplitFetcherManager<>(
- config.getInteger(SourceConfig.NUM_FETCHERS),
- elementsQueue,
+ config.get(SourceConfig.NUM_FETCHERS),
splitFetcherSupplier),
Review Comment:
thank you for pointing it out. Fixed
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@flink.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [FLINK-36745][docs] Make FixedSizeSplitFetcherManager and FixedFetcherSizeSourceReader examples match Flink 2.0-preview API [flink]
Posted by "davidradl (via GitHub)" <gi...@apache.org>.
davidradl commented on code in PR #25667:
URL: https://github.com/apache/flink/pull/25667#discussion_r1850415111
##########
docs/content.zh/docs/dev/datastream/sources.md:
##########
@@ -294,9 +294,9 @@ public class FixedSizeSplitFetcherManager<E, SplitT extends SourceSplit>
public FixedSizeSplitFetcherManager(
int numFetchers,
- FutureCompletingBlockingQueue<RecordsWithSplitIds<E>> elementsQueue,
- Supplier<SplitReader<E, SplitT>> splitReaderSupplier) {
- super(elementsQueue, splitReaderSupplier);
+ Supplier<SplitReader<E, SplitT>> splitReaderSupplier,
+ Configuration config) {
Review Comment:
the Chinese version contains : `SourceReaderContext context) {` as the last parameter. Which one is correct?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@flink.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [FLINK-36745][docs] Make FixedSizeSplitFetcherManager and FixedFetcherSizeSourceReader examples match Flink 2.0-preview API [flink]
Posted by "davidradl (via GitHub)" <gi...@apache.org>.
davidradl commented on PR #25667:
URL: https://github.com/apache/flink/pull/25667#issuecomment-2490395974
Reviewed by Chi on 21/11/24 Need a committer to review
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@flink.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [FLINK-36745][docs] Make FixedSizeSplitFetcherManager and FixedFetcherSizeSourceReader examples match Flink 2.0-preview API [flink]
Posted by "lvyanquan (via GitHub)" <gi...@apache.org>.
lvyanquan commented on code in PR #25667:
URL: https://github.com/apache/flink/pull/25667#discussion_r1849637494
##########
docs/content/docs/dev/datastream/sources.md:
##########
@@ -325,16 +325,13 @@ public class FixedFetcherSizeSourceReader<E, T, SplitT extends SourceSplit, Spli
extends SourceReaderBase<E, T, SplitT, SplitStateT> {
public FixedFetcherSizeSourceReader(
- FutureCompletingBlockingQueue<RecordsWithSplitIds<E>> elementsQueue,
Supplier<SplitReader<E, SplitT>> splitFetcherSupplier,
RecordEmitter<E, T, SplitStateT> recordEmitter,
Configuration config,
SourceReaderContext context) {
super(
- elementsQueue,
new FixedSizeSplitFetcherManager<>(
- config.getInteger(SourceConfig.NUM_FETCHERS),
- elementsQueue,
+ config.get(SourceConfig.NUM_FETCHERS),
splitFetcherSupplier),
Review Comment:
Configuration parameter was not passed to FixedSizeSplitFetcherManager.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@flink.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [FLINK-36745][docs] Make FixedSizeSplitFetcherManager and FixedFetcherSizeSourceReader examples match Flink 2.0-preview API [flink]
Posted by "davidradl (via GitHub)" <gi...@apache.org>.
davidradl commented on PR #25667:
URL: https://github.com/apache/flink/pull/25667#issuecomment-2490395429
Reviewed by Chi on 21/11/24 Need a committer to review
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@flink.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [FLINK-36745][docs] Make FixedSizeSplitFetcherManager and FixedFetcherSizeSourceReader examples match Flink 2.0-preview API [flink]
Posted by "rmetzger (via GitHub)" <gi...@apache.org>.
rmetzger merged PR #25667:
URL: https://github.com/apache/flink/pull/25667
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@flink.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [FLINK-36745][docs] Make FixedSizeSplitFetcherManager and FixedFetcherSizeSourceReader examples match Flink 2.0-preview API [flink]
Posted by "lvyanquan (via GitHub)" <gi...@apache.org>.
lvyanquan commented on code in PR #25667:
URL: https://github.com/apache/flink/pull/25667#discussion_r1850103994
##########
docs/content/docs/dev/datastream/sources.md:
##########
@@ -325,16 +325,13 @@ public class FixedFetcherSizeSourceReader<E, T, SplitT extends SourceSplit, Spli
extends SourceReaderBase<E, T, SplitT, SplitStateT> {
public FixedFetcherSizeSourceReader(
- FutureCompletingBlockingQueue<RecordsWithSplitIds<E>> elementsQueue,
Supplier<SplitReader<E, SplitT>> splitFetcherSupplier,
RecordEmitter<E, T, SplitStateT> recordEmitter,
Configuration config,
SourceReaderContext context) {
super(
- elementsQueue,
new FixedSizeSplitFetcherManager<>(
- config.getInteger(SourceConfig.NUM_FETCHERS),
- elementsQueue,
+ config.get(SourceConfig.NUM_FETCHERS),
splitFetcherSupplier),
Review Comment:
The construction method of FixedSizeSplitFetcherManager is
```
public FixedSizeSplitFetcherManager(
int numFetchers,
Supplier<SplitReader<E, SplitT>> splitReaderSupplier,
Configuration config)
```
but here you created it with
```
new FixedSizeSplitFetcherManager<>(
config.get(SourceConfig.NUM_FETCHERS),
splitFetcherSupplier)
```
config should be passed to construction method too.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@flink.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [FLINK-36745][docs] Make FixedSizeSplitFetcherManager and FixedFetcherSizeSourceReader examples match Flink 2.0 API [flink]
Posted by "flinkbot (via GitHub)" <gi...@apache.org>.
flinkbot commented on PR #25667:
URL: https://github.com/apache/flink/pull/25667#issuecomment-2483837924
<!--
Meta data
{
"version" : 1,
"metaDataEntries" : [ {
"hash" : "08b0bfacabc6726420539105e3071e6f4e9f1dcd",
"status" : "UNKNOWN",
"url" : "TBD",
"triggerID" : "08b0bfacabc6726420539105e3071e6f4e9f1dcd",
"triggerType" : "PUSH"
} ]
}-->
## CI report:
* 08b0bfacabc6726420539105e3071e6f4e9f1dcd UNKNOWN
<details>
<summary>Bot commands</summary>
The @flinkbot bot supports the following commands:
- `@flinkbot run azure` re-run the last Azure build
</details>
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@flink.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org