New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Overriding property `host` & `port` from arquillian configuration if wildfly is binded to 0.0.0.0 #108

Merged
merged 1 commit into from Jun 14, 2017

Conversation

Projects
None yet
3 participants
@dipak-pawar
Contributor

dipak-pawar commented May 19, 2017

Fixes https://issues.jboss.org/browse/WFARQ-30

Now you can use following configuration in arquillian.xml to override host if wildfly is binded to 0.0.0.0 so you can run test as @RunAsClient

  <container qualifier="remote-container" default="true">
    <configuration>
      <property name="managementPort">9991</property>
      <property name="host">192.168.165.37</property>
      <property name="port">8097</property>
    </configuration>

  </container>
Show outdated Hide outdated ...on/src/main/java/org/jboss/as/arquillian/container/ManagementClient.java
@@ -344,14 +347,19 @@ private URI getBinding(final String protocol, final String socketBinding) {
}
}
static String formatIP(String ip) {
static String formatIP(String ip, String host) {

This comment has been minimized.

@jamezp

jamezp May 23, 2017

Member

We should only be formatting a single IP here. The logic for testing which IP needs to be elsewhere.

@jamezp

jamezp May 23, 2017

Member

We should only be formatting a single IP here. The logic for testing which IP needs to be elsewhere.

public void setHost(String host) {
this.host = host;
}

This comment has been minimized.

@jamezp

jamezp May 23, 2017

Member

We should also allow for a port override.

@jamezp

jamezp May 23, 2017

Member

We should also allow for a port override.

This comment has been minimized.

@dipak-pawar

dipak-pawar May 23, 2017

Contributor

I don't see any benefit by port overriding, because we are finding port on the fly using bound-port. You can check it here.

So I am not sure about do we have any valid real use case where we need port overriding?

@dipak-pawar

dipak-pawar May 23, 2017

Contributor

I don't see any benefit by port overriding, because we are finding port on the fly using bound-port. You can check it here.

So I am not sure about do we have any valid real use case where we need port overriding?

This comment has been minimized.

@jamezp

jamezp May 23, 2017

Member

The port can only be found if you query the binding group. There is no guarantee the binding group and the IP passed in will be the same. If there are multiple binding groups this may be using the defined IP, but a different port than the defined IP is listening on.

@jamezp

jamezp May 23, 2017

Member

The port can only be found if you query the binding group. There is no guarantee the binding group and the IP passed in will be the same. If there are multiple binding groups this may be using the defined IP, but a different port than the defined IP is listening on.

This comment has been minimized.

@dipak-pawar

dipak-pawar May 26, 2017

Contributor

@jamezp I have update PR with overwriting port from configuration.

@dipak-pawar

dipak-pawar May 26, 2017

Contributor

@jamezp I have update PR with overwriting port from configuration.

@dipak-pawar dipak-pawar changed the title from Overriding property `host` from arquillian configuration if wildfly is binded to 0.0.0.0 to Overriding property `host` & `port` from arquillian configuration if wildfly is binded to 0.0.0.0 May 26, 2017

@dipak-pawar

This comment has been minimized.

Show comment
Hide comment
@dipak-pawar

dipak-pawar May 31, 2017

Contributor

@jamezp , can you please look into this?

Contributor

dipak-pawar commented May 31, 2017

@jamezp , can you please look into this?

@bartoszmajsak

This comment has been minimized.

Show comment
Hide comment
@bartoszmajsak

bartoszmajsak Jun 12, 2017

Contributor

Hi @jamezp can you check if there is anything else from our side to make this PR merged? Would be great to have this fix and release soon :)

Contributor

bartoszmajsak commented Jun 12, 2017

Hi @jamezp can you check if there is anything else from our side to make this PR merged? Would be great to have this fix and release soon :)

@jamezp

This comment has been minimized.

Show comment
Hide comment
@jamezp

jamezp Jun 12, 2017

Member

I'll try to get this merged this week and cut a release.

Member

jamezp commented Jun 12, 2017

I'll try to get this merged this week and cut a release.

@bartoszmajsak

This comment has been minimized.

Show comment
Hide comment
@bartoszmajsak

bartoszmajsak Jun 12, 2017

Contributor

Awesome! Thanks a lot!

Contributor

bartoszmajsak commented Jun 12, 2017

Awesome! Thanks a lot!

@jamezp

jamezp approved these changes Jun 14, 2017

@jamezp jamezp merged commit a24c820 into wildfly:master Jun 14, 2017

@jamezp

This comment has been minimized.

Show comment
Hide comment
@jamezp

jamezp Jun 14, 2017

Member

I made a couple small changes to the commit before merging. All looks good though thanks!

Member

jamezp commented Jun 14, 2017

I made a couple small changes to the commit before merging. All looks good though thanks!

@jamezp

This comment has been minimized.

Show comment
Hide comment
@jamezp

jamezp Jun 19, 2017

Member

I forgot to mention on Friday I did a 2.1.0.Alpha3 release with this change.

Member

jamezp commented Jun 19, 2017

I forgot to mention on Friday I did a 2.1.0.Alpha3 release with this change.

@dipak-pawar

This comment has been minimized.

Show comment
Hide comment
@dipak-pawar

dipak-pawar Jun 19, 2017

Contributor

Thanks @jamezp

Contributor

dipak-pawar commented Jun 19, 2017

Thanks @jamezp

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment