Skip to content
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

add public address handling to PNP #1334

Merged
merged 2 commits into from
May 6, 2017

Conversation

fviale
Copy link
Member

@fviale fviale commented Apr 28, 2017

  • Added proactive.net.public_address setting. It allows to specify a hostname or ip address which can be used to contact the Remote Objects.
  • This configuration is implemented in the PNP protocol only. In case this setting is configured, the PNP protocol creates urls like pnp://public_address@private_address:port
  • To contact a Remote Object which contains a public address, the pnp protocol tries first to open a socket to the private address, and if the connection fails, it will create a socket to the public address.
  • URIBuilder methods now have userInfo and query parameters
  • Better escaping of urls in jmx FactoryName
  • added a functional test TestPNPPublicAddress

 - Added proactive.net.public_address setting. It allows to specify a hostname or ip address which can be used to contact the Remote Objects.
 - This configuration is implemented in the PNP protocol only. In case this setting is configured, the PNP protocol creates urls like pnp://public_address@private_address:port
 - To contact a Remote Object which contains a public address, the pnp protocol tries first to open a socket to the private address, and if the connection fails, it will create a socket to the public address.
 - URIBuilder methods now have userInfo and query parameters
 - Better escaping of urls in jmx FactoryName
 - added a functional test TestPNPPublicAddress
@fviale fviale self-assigned this Apr 28, 2017
@vinseon vinseon self-requested a review April 28, 2017 15:44
Copy link

@vinseon vinseon left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice improvements overall

@@ -230,7 +231,7 @@ public InternalRemoteRemoteObject createRemoteObject(RemoteObject<?> remoteObjec
}

URI uri = new URI(this.getProtocolId(),
null,
ProActiveInet.getPublicAddress(),
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think we should explicitly indicate here that the public IP address is intentionally given through the "UserInfo" field and explain why (namespace issue using the 'query' field ?).

@fviale
Copy link
Member Author

fviale commented May 2, 2017

Retest this please

Copy link
Contributor

@paraita paraita left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Works but I had to run the spotlessApply task on my machine for the tests to pass

- added a comment which describes the use of userinfo in the pnp uri
- use override annotation
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants