[GitHub] flink pull request #4979: RMQSource support disabling queue declaration
Github user sihuazhou closed the pull request at: https://github.com/apache/flink/pull/4979 ---
[GitHub] flink pull request #4979: RMQSource support disabling queue declaration
Github user tzulitai commented on a diff in the pull request: https://github.com/apache/flink/pull/4979#discussion_r164086981 --- Diff: flink-connectors/flink-connector-rabbitmq/src/main/java/org/apache/flink/streaming/connectors/rabbitmq/RMQSource.java --- @@ -138,7 +138,9 @@ protected ConnectionFactory setupConnectionFactory() throws Exception { * defining custom queue parameters) */ protected void setupQueue() throws IOException { - channel.queueDeclare(queueName, true, false, false, null); + if (rmqConnectionConfig.isQueueDeclaration()) { --- End diff -- I think @GJL's comment is quite valid. It would also mean that the change in this PR is not required for the functionality you want. Do you agree, @sihuazhou? If yes, we can probably close this PR, and the corresponding JIRA. ---
[GitHub] flink pull request #4979: RMQSource support disabling queue declaration
Github user GJL commented on a diff in the pull request: https://github.com/apache/flink/pull/4979#discussion_r162632757 --- Diff: flink-connectors/flink-connector-rabbitmq/src/main/java/org/apache/flink/streaming/connectors/rabbitmq/RMQSource.java --- @@ -138,7 +138,9 @@ protected ConnectionFactory setupConnectionFactory() throws Exception { * defining custom queue parameters) */ protected void setupQueue() throws IOException { - channel.queueDeclare(queueName, true, false, false, null); + if (rmqConnectionConfig.isQueueDeclaration()) { --- End diff -- Thanks for your contribution to Apache Flink @sihuazhou! I have reviewed your code, and I am not sure if the additional flag is needed. The original author of the `RMQSource` declared this method protected, which means that if you do not want the queue to be declared, you can simply override the method with an empty implementation. For example: ``` env.addSource(new RMQSource( connectionConfig, "queueName", true, new SimpleStringSchema()) { @Override protected void setupQueue() { // do not declare queue } }); ``` This intent is also reflected in the Javadoc: ``` /** * Sets up the queue. The default implementation just declares the queue. The user may override * this method to have a custom setup for the queue (i.e. binding the queue to an exchange or * defining custom queue parameters) */ ``` Moreover, `RMQSink#setupQueue` also declares the queue by default, which is not addressed in your pull request. Please let me know what you think @sihuazhou cc: @tzulitai @zentol ---
[GitHub] flink pull request #4979: RMQSource support disabling queue declaration
GitHub user sihuazhou opened a pull request: https://github.com/apache/flink/pull/4979 RMQSource support disabling queue declaration ## What is the purpose of the change This PR fixs [FLINK-8018](https://issues.apache.org/jira/browse/FLINK-8018), RabbitMQ connector should support disabling the call of queueDeclare or not, in case that user does not have sufficient authority to declare the queue. ## Brief change log - *Add queueDeclaration in RMQConnectionConfig to support enable or disable queue declaration, the default value is true* ## Verifying this change This is a trivial change. ## 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, Yarn/Mesos, ZooKeeper: (no) ## Documentation - Does this pull request introduce a new feature? (no) You can merge this pull request into a Git repository by running: $ git pull https://github.com/sihuazhou/flink RMQ_disable_queuedeclare Alternatively you can review and apply these changes as the patch at: https://github.com/apache/flink/pull/4979.patch To close this pull request, make a commit to your master/trunk branch with (at least) the following in the commit message: This closes #4979 commit ae69a201e863eb21b5cf083d05430fe344ed8342 Author: summerleafs Date: 2017-11-08T06:00:55Z introduce queueDeclaration for RMQConnectionConfig. commit 4f4fb71aba2be312829f00ced6801e3439e67533 Author: summerleafs Date: 2017-11-08T06:32:19Z fix build. commit a41b495715acbfd4251f65aa2d023c90e1a7bb94 Author: summerleafs Date: 2017-11-08T06:39:50Z set queueDeclaration default value to true. ---