{"action":"created","issue":{"url":"https://api.github.com/repos/mercury-hpc/mercury/issues/112","repository_url":"https://api.github.com/repos/mercury-hpc/mercury","labels_url":"https://api.github.com/repos/mercury-hpc/mercury/issues/112/labels{/name}","comments_url":"https://api.github.com/repos/mercury-hpc/mercury/issues/112/comments","events_url":"https://api.github.com/repos/mercury-hpc/mercury/issues/112/events","html_url":"https://github.com/mercury-hpc/mercury/pull/112","id":161766916,"number":112,"title":"[WIP] address parsing","user":{"login":"JohnPJenkins","id":8118010,"avatar_url":"https://avatars.githubusercontent.com/u/8118010?v=3","gravatar_id":"","url":"https://api.github.com/users/JohnPJenkins","html_url":"https://github.com/JohnPJenkins","followers_url":"https://api.github.com/users/JohnPJenkins/followers","following_url":"https://api.github.com/users/JohnPJenkins/following{/other_user}","gists_url":"https://api.github.com/users/JohnPJenkins/gists{/gist_id}","starred_url":"https://api.github.com/users/JohnPJenkins/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/JohnPJenkins/subscriptions","organizations_url":"https://api.github.com/users/JohnPJenkins/orgs","repos_url":"https://api.github.com/users/JohnPJenkins/repos","events_url":"https://api.github.com/users/JohnPJenkins/events{/privacy}","received_events_url":"https://api.github.com/users/JohnPJenkins/received_events","type":"User","site_admin":false},"labels":[],"state":"open","locked":false,"assignee":null,"assignees":[],"milestone":null,"comments":12,"created_at":"2016-06-22T19:34:01Z","updated_at":"2016-06-30T19:51:15Z","closed_at":null,"pull_request":{"url":"https://api.github.com/repos/mercury-hpc/mercury/pulls/112","html_url":"https://github.com/mercury-hpc/mercury/pull/112","diff_url":"https://github.com/mercury-hpc/mercury/pull/112.diff","patch_url":"https://github.com/mercury-hpc/mercury/pull/112.patch"},"body":"Here's an initial look to get an idea of what my address parsing changes would do. Strategy is to parse through the class+protocol pair, then leave the remainder for the NA layer. MPI and CCI already ignore the host name, so there was not much to do there. BMI requires reconstructing the \"protocol://hostname:port\", but that's a fairly simple piece of code.\r\n\r\nNote that addr_lookup in the top-level NA layer just peeks at the class name, and passes the remainder down to the plugin. The BMI and CCI plugins then pass them directly to the libs, while MPI parses the port string starting from ';', effectively ignoring the \"protocol://\" portion.\r\n\r\nTODO:\r\n\r\n- [x] Modify tests to init clients in anon mode (\"class+protocol\"). Currently passes in the *server* string to initialize.\r\n- [x] ~~Modify tests to always get the server address through file. Right now it does sometimes but not others. Will be simpler to just have one code path, then we can test things like having anon-mode servers.~~ This actually isn't the case, I misread the code.\r\n- [x] Get rid of hard-coded tcp default protocol in tests (or just special case MPI to \"dynamic\") - use the NA_*_TESTING_PROTOCOL variables for this instead.\r\n- [x] Get rid of default protocol check in test code, enforce given protocol\r\n- [x] Turn MPI error checks (bad protocol, existence of hostname during initialization) into errors once tests are changed. They are currently warnings.\r\n- [x] Rebase on Phil's cci_create_endpoint_at branch once it gets merged, and incorporate call into code.\r\n- [x] Special-case address creation in test code. BMI, CCI+tcp(+verbs?) - hostname:port. CCI+sm - pid:tag (need to check format). MPI: nothing. Right now, hardcodes host string to hostname:port unless using MPI.\r\n- [x] ~~Similarly, modify BMI to allow anon-mode servers? This might require a BMI change, if so, then just error out when no hoststring is provided at the NA layer.~~ Not worth the trouble.\r\n\r\nMight have forgot one or two..."},"comment":{"url":"https://api.github.com/repos/mercury-hpc/mercury/issues/comments/229769073","html_url":"https://github.com/mercury-hpc/mercury/pull/112#issuecomment-229769073","issue_url":"https://api.github.com/repos/mercury-hpc/mercury/issues/112","id":229769073,"user":{"login":"JohnPJenkins","id":8118010,"avatar_url":"https://avatars.githubusercontent.com/u/8118010?v=3","gravatar_id":"","url":"https://api.github.com/users/JohnPJenkins","html_url":"https://github.com/JohnPJenkins","followers_url":"https://api.github.com/users/JohnPJenkins/followers","following_url":"https://api.github.com/users/JohnPJenkins/following{/other_user}","gists_url":"https://api.github.com/users/JohnPJenkins/gists{/gist_id}","starred_url":"https://api.github.com/users/JohnPJenkins/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/JohnPJenkins/subscriptions","organizations_url":"https://api.github.com/users/JohnPJenkins/orgs","repos_url":"https://api.github.com/users/JohnPJenkins/repos","events_url":"https://api.github.com/users/JohnPJenkins/events{/privacy}","received_events_url":"https://api.github.com/users/JohnPJenkins/received_events","type":"User","site_admin":false},"created_at":"2016-06-30T19:51:15Z","updated_at":"2016-06-30T19:51:15Z","body":"I've fixed up the test address generation code and tests are back to green locally. Before a pull happens, want to summarize where we are and propose a simplification.\r\n\r\n# Initialization\r\nWhen not listening, any plugin can pass in an address string of the form `[<class>+]<protocol>[://]\". I.e. no hostname/port expected. (the `[]` means optional here).\r\n- BMI ignores passed hostnames AFAICT.\r\n- CCI will try to create its endpoint with whatever you give it, so non-listeners can request a specific port, for example.\r\n- MPI expects no hostname and errors out otherwise.\r\n\r\nIn listen mode, the expected hostname is plugin dependent.\r\n- BMI expects `<host>:<port>`\r\n- CCI falls back to cci_create_endpoint when a host string isn't provided, using an OS assigned port for TCP,Verbs and the process PID for SM.\r\n- CCI+TCP,Verbs expect `<port>` if an addr is provided.\r\n- CCI+SM expect `<id1>:<id2>` if an addr is provided.\r\n\r\nWe can simplify the BMI,CCI split by changing the BMI NA to just expect a port and appending \"localhost\" under the covers, as there's no other valid option. Thoughts?\r\n\r\n# Lookup\r\nNothing has changed here. Class still optional, and \"full\" host strings are expected."}}
{"actor":{"display_login":"JohnPJenkins"}}