Plasma photon association: passing through plasma address with photon db connection #123
Conversation
| @@ -243,7 +243,8 @@ def start_ray_local(node_ip_address="127.0.0.1", num_workers=0, num_local_schedu | |||
| object_store_manager_names.append(object_store_manager_name) | |||
| time.sleep(0.1) | |||
| # Start the local scheduler. | |||
| local_scheduler_name = start_local_scheduler(redis_address, object_store_name, object_store_manager_name, cleanup=True) | |||
| plasma_address = "%s:%s" %(node_ip_address, object_store_manager_port) | |||
There was a problem hiding this comment.
can you use "{}:{}".format(node_ip_address, object_store_manager_port) here instead to be consistent with the rest of the python codebase?
| CHECK(reply); | ||
| if (reply->type != REDIS_REPLY_NIL) { | ||
| freeReplyObject(reply); | ||
| break; | ||
| } | ||
| freeReplyObject(reply); | ||
| printf("redis_db_connect: end of while loop\n"); |
| (char *) client.id, sizeof(client.id), client_type, | ||
| client_addr, client_port, (char *) client.id, | ||
| sizeof(client.id), aux_address); | ||
| CHECK_REDIS_CONNECT(redisContext, context, |
There was a problem hiding this comment.
this should be just CHECKM(reply != NULL, "db_connect failed on HMSET");
The CHECK_REDIS_CONNECT is only for redisConnect commands, not for redisCommand
There was a problem hiding this comment.
actually, CHECK_REDIS_CONNECT macro is more general than its name suggests. It can check any redis context for an error code.
There was a problem hiding this comment.
we should probably rename the macro.
| CHECKM(reply != NULL, "db_connect failed on HMSET"); | ||
| /* allocate space for the published message */ | ||
| char *tmpbuf = | ||
| malloc(strlen(client_type) + 1 + strlen(aux_address + 1 + 1)); |
There was a problem hiding this comment.
| int rv = sscanf(1 + payload->str + sizeof(client.id), "%s %s", client_type, | ||
| aux_address); | ||
| if (rv < 2) { | ||
| /* Backward-compatibility: try to extract client_type only. */ |
There was a problem hiding this comment.
Can't we just omit that? We don't need to be backwards compatible (yet), right?
There was a problem hiding this comment.
it was an attempt to handle the case when we parse fewer than expected number of string tokens. But I'll replace that with an explicit assert.
| char *aux_address = malloc(client_type_length); | ||
| memset(aux_address, 0, client_type_length); | ||
| /* Published message format: <client_id:client_type aux_addr> */ | ||
| int rv = sscanf(1 + payload->str + sizeof(client.id), "%s %s", client_type, |
There was a problem hiding this comment.
&payload->str[1 + sizeof(client.id)]
atumanov
force-pushed
the
plasma-photon-association
branch
2 times, most recently
from
cd04142 to
49d43e6
Compare
atumanov
force-pushed
the
plasma-photon-association
branch
from
a4d832a to
1c99f58
Compare
No description provided.