[GitHub] flink pull request #4979: RMQSource support disabling queue declaration

[sihuazhou]

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

[tzulitai]

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

[GJL]

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

[sihuazhou]

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.




---