Provide separate module with Pulsar v1 client API wrapper #3228
Conversation
merlimat
added
the
type/enhancement
merlimat
force-pushed
the
separate-api-from-impl
branch
from
07f6222 to
1f4596a
Compare
|
run integration tests |
pulsar-client/src/test/java/org/apache/pulsar/client/impl/schema/DefaultSchemasTest.java
Show resolved
Hide resolved
|
looks good to me |
| import org.apache.pulsar.client.api.MessageId; | ||
| import org.apache.pulsar.client.api.PulsarClientException; | ||
|
|
||
| public class ConsumerV1Impl implements Consumer { |
There was a problem hiding this comment.
if package name already has v1 then we should not add V1 into class name and it's better to keep ConsumerImpl.
There was a problem hiding this comment.
This is to avoid confusion in the IDE with classes with same name in different namespaces
There was a problem hiding this comment.
This is to avoid confusion in the IDE with classes with same name in different namespaces
Umm.. I don't think it creates confusion and it's better to continue the same name with version(v1,v2) present into namespace.
| <relativePath>..</relativePath> | ||
| </parent> | ||
|
|
||
| <artifactId>pulsar-client-2x-shaded</artifactId> |
There was a problem hiding this comment.
this is confusing. pulsar-client-1x-base has child module pulsar-client-2x-shaded ? client-2.x already has separate module at parent level. Shouldn't we need pulsar-client-1x-shaded ?
There was a problem hiding this comment.
The problem I'm trying to solve here is that pulsar-client-1x is a wrapper over pulsar-client. At the same time, we have some interfaces that need to be duplicated in pulsar-client-1x (because the 2.x version will be different).
So, pulsar-client-2x-shaded is really pulsar-client but with just few interfaces renamed (eg: PulsarClient, Producer, Consumer and Reader) to avoid conflicts with the 1.x API.
So, although confusing, pulsar-client-2x-shaded is really the correct name and this module is not meant for direct usage anyway.
There was a problem hiding this comment.
It raises few questions for the clients who are using V1 api and not ready to move V2 yet but still want to use new features.
- are we going to maintain this V1 api artifact with the new configuration changes?
- it will enforce client app to change artifact dependency else their existing app will fail.
So, instead can't we keep this api into the same pulsar-client artifact because anyway it doesn't have many classes and it will be easy to maintain if we have everything under one location.
In every system you need to expect to upgrade and follow newer best practices in order to use new features. That is unrelated to the compatibility of older client library which is out of question.
Anyone interested in adding the new features in the v1 API can still update the 1x wrapper and add them. It will be trivial to do so.
An application is not "forced" to upgrade the artifact dependency. It might want to upgrade to get new bug or security fixes. In that case it will have to update the Maven dependency (the version, at least). If the application is using the v1 API, it will have to switch to use To summarize:
The purpose of marking API methods as "Deprecated" is to give a warning and remove them in the future. The current state of things is very confusing because we have 2 sets of APIs that a user have to chose from. This adds a lot to the knowledge required to get started with the API. There are additional issue in the current state:
Older API has a mix of interfaces and POJO which access concrete classes in other package. That kind of derailed over time and this is a way to fix that, do the cleanup and expose a very clean API surface. |
|
@merlimat agreed |
* After #3228, removed usages of deprecated client API * Removed ProducerConfiguration from flink connector * Use pulsar-client-1x in pulsar-spark * Fixed log4j adapter * Fixed flink examples * Fixed storm integration tests * Fixed storm example * Wrapping message listeners in v1 lib wrapper * Fixed unit tests * Moved spark receiver test to test-containers to avoid shading issues * Fixed PulsarAvroTableSinkTest * Only run spark receiver test in integration tests
Motivation
Provide a Pulsar V1 API compatibility wrappers.
Applications can use artifact name
pulsar-client-1xinstead ofpulsar-clientand this will allow us to drop the deprecated methods frompulsar-client.