You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@plc4x.apache.org by "splatch (via GitHub)" <gi...@apache.org> on 2025/03/03 19:46:51 UTC
[PR] feat(plc4j/opcua): Remove additional hostname resolution within OPC-UA
splatch opened a new pull request, #2028:
URL: https://github.com/apache/plc4x/pull/2028
Closes #1897. Removed code which caused additional DNS lookups in favor of endpoint-port and endpoint-host options provided with plc4j 0.13.
--
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: dev-unsubscribe@plc4x.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] feat(plc4j/opcua): Remove additional hostname resolution within OPC-UA
Posted by "sruehl (via GitHub)" <gi...@apache.org>.
sruehl commented on code in PR #2028:
URL: https://github.com/apache/plc4x/pull/2028#discussion_r1981928017
##########
plc4j/drivers/opcua/src/test/java/org/apache/plc4x/java/opcua/OpcuaPlcDriverTest.java:
##########
@@ -516,21 +519,24 @@ public void run() {
PlcReadResponse read_response = read_request.execute().get();
assertThat(read_response.getResponseCode("Bool")).isEqualTo(PlcResponseCode.OK);
}
-
+ this.latch.countDown();
} catch (ExecutionException e) {
LOGGER.error("run aborted", e);
+ this.latch.countDown();
} catch (InterruptedException e) {
- Thread.currentThread().interrupt();
Review Comment:
because this ensures that the interrupt flag stays set https://github.com/joansmith/sonar-java/blob/master/java-checks/src/main/resources/org/sonar/l10n/java/rules/squid/S2142.html
--
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: dev-unsubscribe@plc4x.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] feat(plc4j/opcua): Remove additional hostname resolution within OPC-UA
Posted by "sruehl (via GitHub)" <gi...@apache.org>.
sruehl merged PR #2028:
URL: https://github.com/apache/plc4x/pull/2028
--
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: dev-unsubscribe@plc4x.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] feat(plc4j/opcua): Remove additional hostname resolution within OPC-UA
Posted by "sruehl (via GitHub)" <gi...@apache.org>.
sruehl commented on code in PR #2028:
URL: https://github.com/apache/plc4x/pull/2028#discussion_r1981381035
##########
plc4j/drivers/opcua/src/test/java/org/apache/plc4x/java/opcua/OpcuaPlcDriverTest.java:
##########
@@ -516,21 +519,24 @@ public void run() {
PlcReadResponse read_response = read_request.execute().get();
assertThat(read_response.getResponseCode("Bool")).isEqualTo(PlcResponseCode.OK);
}
-
+ this.latch.countDown();
} catch (ExecutionException e) {
LOGGER.error("run aborted", e);
+ this.latch.countDown();
} catch (InterruptedException e) {
- Thread.currentThread().interrupt();
Review Comment:
this must stay in
##########
plc4j/drivers/opcua/src/test/java/org/apache/plc4x/java/opcua/OpcuaPlcDriverTest.java:
##########
@@ -544,11 +550,13 @@ public void run() {
PlcWriteResponse write_response = write_request.execute().get();
assertThat(write_response.getResponseCode("Bool")).isEqualTo(PlcResponseCode.OK);
}
+ this.latch.countDown();
} catch (ExecutionException e) {
LOGGER.error("run aborted", e);
+ this.latch.countDown();
} catch (InterruptedException e) {
- Thread.currentThread().interrupt();
Review Comment:
this must stay in
--
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: dev-unsubscribe@plc4x.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] feat(plc4j/opcua): Remove additional hostname resolution within OPC-UA
Posted by "sruehl (via GitHub)" <gi...@apache.org>.
sruehl commented on PR #2028:
URL: https://github.com/apache/plc4x/pull/2028#issuecomment-2700920063
other than that I don't have any objections
--
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: dev-unsubscribe@plc4x.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] feat(plc4j/opcua): Remove additional hostname resolution within OPC-UA
Posted by "splatch (via GitHub)" <gi...@apache.org>.
splatch commented on code in PR #2028:
URL: https://github.com/apache/plc4x/pull/2028#discussion_r1987891731
##########
plc4j/drivers/opcua/src/test/java/org/apache/plc4x/java/opcua/OpcuaPlcDriverTest.java:
##########
@@ -516,21 +519,24 @@ public void run() {
PlcReadResponse read_response = read_request.execute().get();
assertThat(read_response.getResponseCode("Bool")).isEqualTo(PlcResponseCode.OK);
}
-
+ this.latch.countDown();
} catch (ExecutionException e) {
LOGGER.error("run aborted", e);
+ this.latch.countDown();
} catch (InterruptedException e) {
- Thread.currentThread().interrupt();
Review Comment:
Only one case for interrupted exception to happen is when test is terminated before completing 100 read or write attempts. Because exception is caught outside of the loop thread is effectively terminated at the time exception is caught.
--
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: dev-unsubscribe@plc4x.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] feat(plc4j/opcua): Remove additional hostname resolution within OPC-UA
Posted by "splatch (via GitHub)" <gi...@apache.org>.
splatch commented on code in PR #2028:
URL: https://github.com/apache/plc4x/pull/2028#discussion_r1987891731
##########
plc4j/drivers/opcua/src/test/java/org/apache/plc4x/java/opcua/OpcuaPlcDriverTest.java:
##########
@@ -516,21 +519,24 @@ public void run() {
PlcReadResponse read_response = read_request.execute().get();
assertThat(read_response.getResponseCode("Bool")).isEqualTo(PlcResponseCode.OK);
}
-
+ this.latch.countDown();
} catch (ExecutionException e) {
LOGGER.error("run aborted", e);
+ this.latch.countDown();
} catch (InterruptedException e) {
- Thread.currentThread().interrupt();
Review Comment:
Only one case for interrupted exception to happen is when test is terminated before completing 100 read or write attempts, long story short - its already dead. Because exception is caught outside of the loop thread is effectively terminated at the time exception is caught.
--
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: dev-unsubscribe@plc4x.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] feat(plc4j/opcua): Remove additional hostname resolution within OPC-UA
Posted by "splatch (via GitHub)" <gi...@apache.org>.
splatch commented on code in PR #2028:
URL: https://github.com/apache/plc4x/pull/2028#discussion_r1981872417
##########
plc4j/drivers/opcua/src/test/java/org/apache/plc4x/java/opcua/OpcuaPlcDriverTest.java:
##########
@@ -516,21 +519,24 @@ public void run() {
PlcReadResponse read_response = read_request.execute().get();
assertThat(read_response.getResponseCode("Bool")).isEqualTo(PlcResponseCode.OK);
}
-
+ this.latch.countDown();
} catch (ExecutionException e) {
LOGGER.error("run aborted", e);
+ this.latch.countDown();
} catch (InterruptedException e) {
- Thread.currentThread().interrupt();
Review Comment:
Can you explain me why? We get this exception once thread is interrupted from outside, what is the point of calling interrupt again? Especially that in this place we are already out of the loop.
--
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: dev-unsubscribe@plc4x.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] feat(plc4j/opcua): Remove additional hostname resolution within OPC-UA
Posted by "sruehl (via GitHub)" <gi...@apache.org>.
sruehl commented on code in PR #2028:
URL: https://github.com/apache/plc4x/pull/2028#discussion_r1981928486
##########
plc4j/drivers/opcua/src/test/java/org/apache/plc4x/java/opcua/OpcuaPlcDriverTest.java:
##########
@@ -516,21 +519,24 @@ public void run() {
PlcReadResponse read_response = read_request.execute().get();
assertThat(read_response.getResponseCode("Bool")).isEqualTo(PlcResponseCode.OK);
}
-
+ this.latch.countDown();
} catch (ExecutionException e) {
LOGGER.error("run aborted", e);
+ this.latch.countDown();
} catch (InterruptedException e) {
- Thread.currentThread().interrupt();
Review Comment:
Out of curiosity: why did you remove the line?
--
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: dev-unsubscribe@plc4x.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org