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 for node / tcp #6708

Closed
nazmulidris opened this issue Oct 20, 2023 · 1 comment
Closed

Library - Slim down the NodeManagerWorker for node / tcp #6708

nazmulidris opened this issue Oct 20, 2023 · 1 comment
Labels
Component: API ockam_api hacktoberfest Apply to issues you want contributors to help with help wanted Implementation: Rust

Comments

@nazmulidris
Copy link
Contributor

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.

Desired behavior

In this issue handle refactoring the following (each line corresponds to some paths in the handle_request method):

  • node / tcp connection
  • node / tcp listener

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....

Note

Related epic: #6237


We love helping new contributors! ❤️
If you have questions or need help as you explore, please join us on Discord. If you're looking for other issues to contribute to, please checkout our good first issues.

@etorreborre
Copy link
Member

Done with #7040

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Component: API ockam_api hacktoberfest Apply to issues you want contributors to help with help wanted Implementation: Rust
Projects
None yet
Development

No branches or pull requests

2 participants