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

Library - Slim down the NodeManagerWorker #6237

Closed
etorreborre opened this issue Oct 3, 2023 · 0 comments
Closed

Library - Slim down the NodeManagerWorker #6237

etorreborre opened this issue Oct 3, 2023 · 0 comments
Assignees
Labels

Comments

@etorreborre
Copy link
Member

etorreborre commented Oct 3, 2023

The purpose of the NodeManagerWorker is to be started as a Worker on a node and receive requests to create entities on that node: inlets, outlets, secure channels, etc...

This should be the only responsibility of the NodeManagerWorker:

  • accept Requests, with a specific path
  • based on the path, unpack the request arguments and call the relevant method on the NodeManager
  • get a value back from the NodeManager and make a Response out of it

Example

For example if we look at this dispatched request:

(Get, ["node", "tcp", "connection", address]) => {
    encode_request_result(self.get_tcp_connection(req, address.to_string()).await)?
}

The implementation in the NodeManagerWorker is:

pub async fn get_tcp_connection(&self, req: &RequestHeader, address: String) 
    -> Result<Response<TransportStatus>, Response<Error>> {
    let tcp_transport = &self.node_manager.tcp_transport;
    let sender = match Self::find_connection(tcp_transport, address.to_string()) {
        None => {
            return Err(Response::not_found(
                req,
                &format!("Connection {address} was not found in the registry."),
            ));
        }
        Some(sender) => sender,
    };
    let status = TransportStatus::new(ApiTransport {
        tt: TransportType::Tcp,
        tm: (*sender.mode()).into(),
        socket_address: sender.socket_address(),
        worker_address: sender.address().to_string(),
        processor_address: sender.receiver_address().to_string(),
        flow_control_id: sender.flow_control_id().clone(),
    });
    Ok(Response::ok(req).body(status))
}

It should actually be something like:

pub async fn get_tcp_connection(&self, req: &RequestHeader, address: String) 
    -> Result<Response<TransportStatus>, Response<Error>> {
    match &self.node_manager.get_tcp_connection(address.to_string()).await? {
        Some(transport_status) => Ok(Response::ok(req).body(transport_status),
        None => {
            Err(Response::not_found(
                req,
                &format!("Connection {address} was not found in the registry."),
            ))
         }
    }
}

In the code above all the logic is handled by the NodeManager and the NodeManagerWorker only deals with Request/Responses.

Stories

The following independent stories can be carved out this epic (each line corresponds to some paths in the handle_request method):

  • node / node status
  • node / tcp connection
  • node / tcp listener
  • node / credentials
  • node / secure channel
  • node / secure channel listener
  • node / services / kafka
  • node / services / other services
  • node / inlet
  • node / outlet
  • node / flow controls
  • node / workers
  • policy

Additional notes

You can follow the way relays are handled in this PR (here is the section on relays).

You will notice that the NodeManagerWorker does not contain a direct reference to a NodeManager but to an InMemoryNode, which itself contains a NodeManager.

This intermediary layer is only required to monitor sessions for relays and portals. In most cases when you call self.node_manager in NodeManagerWorker the node_manager can be dereferenced to a NodeManager on which regular calls can be made: to list workers, get a credential etc....

Related issues:

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

1 participant