You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@flink.apache.org by "MartijnVisser (via GitHub)" <gi...@apache.org> on 2023/05/11 18:29:23 UTC
[GitHub] [flink] MartijnVisser opened a new pull request, #22571: [FLINK-32056][Python][Connector/Pulsar] Upgrade flink-connector-pulsar in flink-python to v4.0.0
MartijnVisser opened a new pull request, #22571:
URL: https://github.com/apache/flink/pull/22571
## What is the purpose of the change
* Upgrade used `flink-connector-pulsar` in `flink-python` to 4.0.0, since that's compatible with Flink 1.17+. Without this upgrade, we can't upgrade `flink-shaded` since the older versions of `flink-connector-pulsar` directly use `flink-shaded` and there's a breaking change in `flink-shaded`.
## Brief change log
* Upgrade to `flink-connector-pulsar` to 4.0.0
* Remove and refactoring all occurrences of `PulsarSerializationSchema`
## Verifying this change
This change is a trivial rework / code cleanup without any test coverage.
## Does this pull request potentially affect one of the following parts:
- Dependencies (does it add or upgrade a dependency): yes
- 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
[GitHub] [flink] dianfu commented on a diff in pull request #22571: [FLINK-32056][Python][Connector/Pulsar] Upgrade flink-connector-pulsar in flink-python to v4.0.0
Posted by "dianfu (via GitHub)" <gi...@apache.org>.
dianfu commented on code in PR #22571:
URL: https://github.com/apache/flink/pull/22571#discussion_r1191830067
##########
flink-python/pyflink/datastream/connectors/pulsar.py:
##########
@@ -693,8 +668,7 @@ class PulsarSinkBuilder(object):
... .set_service_url(PULSAR_BROKER_URL) \\
... .set_admin_url(PULSAR_BROKER_HTTP_URL) \\
... .set_topics([TOPIC1, TOPIC2]) \\
- ... .set_serialization_schema(
- ... PulsarSerializationSchema.flink_schema(SimpleStringSchema())) \\
+ ... .set_value_serialization_schema(SimpleStringSchema()) \\
Review Comment:
```suggestion
... .set_serialization_schema(SimpleStringSchema()) \\
```
##########
flink-python/pyflink/datastream/connectors/pulsar.py:
##########
@@ -483,18 +438,17 @@ def set_bounded_stop_cursor(self, stop_cursor: StopCursor) -> 'PulsarSourceBuild
self._j_pulsar_source_builder.setBoundedStopCursor(stop_cursor._j_stop_cursor)
return self
- def set_deserialization_schema(self,
- pulsar_deserialization_schema: PulsarDeserializationSchema) \
+ def set_value_only_deserializer(self, deserialization_schema: DeserializationSchema) \
-> 'PulsarSourceBuilder':
"""
- DeserializationSchema is required for getting the Schema for deserialize message from
- pulsar and getting the TypeInformation for message serialization in flink.
+ Sets the :class:`~pyflink.common.serialization.DeserializationSchema` for deserializing the
+ value of Pulsars message.
- We have defined a set of implementations, using PulsarDeserializationSchema#flink_type_info
- or PulsarDeserializationSchema#flink_schema for creating the desired schema.
+ :param deserialization_schema: the :class:`DeserializationSchema` to use for
+ deserialization.
+ :return: this PulsarSourceBuilder.
"""
- self._j_pulsar_source_builder.setDeserializationSchema(
- pulsar_deserialization_schema._j_pulsar_deserialization_schema)
+ self._j_builder.setValueOnlyDeserializer(deserialization_schema._j_deserialization_schema)
Review Comment:
I'm curious why you are trying to call `setValueOnlyDeserializer`. Per my understanding, there is no such interface at all: https://github.com/apache/flink-connector-pulsar/blob/main/flink-connector-pulsar/src/main/java/org/apache/flink/connector/pulsar/source/PulsarSourceBuilder.java
--
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
[GitHub] [flink] dianfu closed pull request #22571: [FLINK-32056][Python][Connector/Pulsar] Upgrade flink-connector-pulsar in flink-python to v4.0.0
Posted by "dianfu (via GitHub)" <gi...@apache.org>.
dianfu closed pull request #22571: [FLINK-32056][Python][Connector/Pulsar] Upgrade flink-connector-pulsar in flink-python to v4.0.0
URL: https://github.com/apache/flink/pull/22571
--
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
[GitHub] [flink] dianfu commented on pull request #22571: [FLINK-32056][Python][Connector/Pulsar] Upgrade flink-connector-pulsar in flink-python to v4.0.0
Posted by "dianfu (via GitHub)" <gi...@apache.org>.
dianfu commented on PR #22571:
URL: https://github.com/apache/flink/pull/22571#issuecomment-1545020164
I noticed that the documentation of 1.17 has already been updated to 4.0.0-1.17, see more details on https://nightlies.apache.org/flink/flink-docs-release-1.17/docs/connectors/datastream/pulsar/
I guess we need also port this to release-1.17?
--
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
[GitHub] [flink] MartijnVisser commented on a diff in pull request #22571: [FLINK-32056][Python][Connector/Pulsar] Upgrade flink-connector-pulsar in flink-python to v4.0.0
Posted by "MartijnVisser (via GitHub)" <gi...@apache.org>.
MartijnVisser commented on code in PR #22571:
URL: https://github.com/apache/flink/pull/22571#discussion_r1200584019
##########
flink-python/pyflink/datastream/connectors/pulsar.py:
##########
@@ -483,18 +438,17 @@ def set_bounded_stop_cursor(self, stop_cursor: StopCursor) -> 'PulsarSourceBuild
self._j_pulsar_source_builder.setBoundedStopCursor(stop_cursor._j_stop_cursor)
return self
- def set_deserialization_schema(self,
- pulsar_deserialization_schema: PulsarDeserializationSchema) \
+ def set_value_only_deserializer(self, deserialization_schema: DeserializationSchema) \
-> 'PulsarSourceBuilder':
"""
- DeserializationSchema is required for getting the Schema for deserialize message from
- pulsar and getting the TypeInformation for message serialization in flink.
+ Sets the :class:`~pyflink.common.serialization.DeserializationSchema` for deserializing the
+ value of Pulsars message.
- We have defined a set of implementations, using PulsarDeserializationSchema#flink_type_info
- or PulsarDeserializationSchema#flink_schema for creating the desired schema.
+ :param deserialization_schema: the :class:`DeserializationSchema` to use for
+ deserialization.
+ :return: this PulsarSourceBuilder.
"""
- self._j_pulsar_source_builder.setDeserializationSchema(
- pulsar_deserialization_schema._j_pulsar_deserialization_schema)
+ self._j_builder.setValueOnlyDeserializer(deserialization_schema._j_deserialization_schema)
Review Comment:
That's just wrong :) Hopefully it's fixed now
--
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
[GitHub] [flink] MartijnVisser commented on pull request #22571: [FLINK-32056][Python][Connector/Pulsar] Upgrade flink-connector-pulsar in flink-python to v4.0.0
Posted by "MartijnVisser (via GitHub)" <gi...@apache.org>.
MartijnVisser commented on PR #22571:
URL: https://github.com/apache/flink/pull/22571#issuecomment-1545224234
> I guess we need also port this to release-1.17?
Yes indeed, because the existing used connector in `release-1.17` actually doesn't work with 1.17
--
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
[GitHub] [flink] dianfu commented on pull request #22571: [FLINK-32056][Python][Connector/Pulsar] Upgrade flink-connector-pulsar in flink-python to v4.0.0
Posted by "dianfu (via GitHub)" <gi...@apache.org>.
dianfu commented on PR #22571:
URL: https://github.com/apache/flink/pull/22571#issuecomment-1544991691
@MartijnVisser Thanks a lot for this work! Besides the sink, I guess we should also update PulsarSourceBuilder.set_deserialization_schema
--
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
[GitHub] [flink] flinkbot commented on pull request #22571: [FLINK-32056][Python][Connector/Pulsar] Upgrade flink-connector-pulsar in flink-python to v4.0.0
Posted by "flinkbot (via GitHub)" <gi...@apache.org>.
flinkbot commented on PR #22571:
URL: https://github.com/apache/flink/pull/22571#issuecomment-1544498302
<!--
Meta data
{
"version" : 1,
"metaDataEntries" : [ {
"hash" : "e142db78820c242c71c56fafecedc9e9bdadb7f7",
"status" : "UNKNOWN",
"url" : "TBD",
"triggerID" : "e142db78820c242c71c56fafecedc9e9bdadb7f7",
"triggerType" : "PUSH"
} ]
}-->
## CI report:
* e142db78820c242c71c56fafecedc9e9bdadb7f7 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