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

Implement timeoutMiddleware and Remove metadata.authorization for src/nodes/agent Domain #572

Closed
addievo opened this issue Oct 12, 2023 · 5 comments · Fixed by #589
Closed
Assignees
Labels
development Standard development

Comments

@addievo
Copy link
Contributor

addievo commented Oct 12, 2023

Issue is WIP

Specification

  1. agentService is a public service, so it should not have authorisation, but in the current implementation, it does. Which is to be altered.
  2. agentService currently does not use timeoutMiddleware, which it should.

Additional context

js-rpc has a sister PR which addresses the same issues - MatrixAI/js-rpc#42

Tasks

  1. Remove authorisation from AgentRPCRequestParams and AgentRPCResponseResult
  2. Implement timeoutMiddleware for agentService
@addievo addievo added the development Standard development label Oct 12, 2023
@addievo addievo self-assigned this Oct 12, 2023
@CMCDragonkai
Copy link
Member

What is AgentService? Does not make sense here.

@addievo
Copy link
Contributor Author

addievo commented Oct 18, 2023

@CMCDragonkai you asked me to write up this issue, its regarding

// Prevent overwriting the metadata type with `Omit<>`
type AgentRPCRequestParams<T extends Record<string, JSONValue> = ObjectEmpty> =
  {
    metadata?: {
      [Key: string]: JSONValue;
    } & Partial<{
      authorization: string;
      timeout: number;
    }>;
  } & Omit<T, 'metadata'>;

// Prevent overwriting the metadata type with `Omit<>`
type AgentRPCResponseResult<T extends Record<string, JSONValue> = ObjectEmpty> =
  {
    metadata?: {
      [Key: string]: JSONValue;
    } & Partial<{
      authorization: string;
      timeout: number;
    }>;
  } & Omit<T, 'metadata'>;

And the fact that AgentService is a publicly accessible service, consequently it should not have an authorisation field.

@CMCDragonkai
Copy link
Member

CMCDragonkai commented Oct 18, 2023

Is AgentService actually a class or type? If not don't refer to it in capitalised form.

@addievo
Copy link
Contributor Author

addievo commented Oct 18, 2023

Is AgentService actually a class or type? If not don't refer to it in capitalised form.

It is not a class, this issue has something to do with the nodes domain, I don't remember that well, I need to go over this with Brian.

@CMCDragonkai
Copy link
Member

Then rename the pr title. It's confusing.

@addievo addievo changed the title Fix: AgentService currently doesn't use timeout middleware and AgentService has authorisation Fix: agentService currently doesn't use timeout middleware and agentService has authorisation Oct 19, 2023
@amydevs amydevs changed the title Fix: agentService currently doesn't use timeout middleware and agentService has authorisation Remove metadata.authorization from src/nodes/agent Domain, and Implement timeoutMiddleware for src/nodes/agent Domain Nov 1, 2023
@amydevs amydevs changed the title Remove metadata.authorization from src/nodes/agent Domain, and Implement timeoutMiddleware for src/nodes/agent Domain Remove Implement timeoutMiddleware and remove metadata.authorization for src/nodes/agent Nov 1, 2023
@amydevs amydevs changed the title Remove Implement timeoutMiddleware and remove metadata.authorization for src/nodes/agent Remove Implement timeoutMiddleware and remove metadata.authorization for src/nodes/agent Domain Nov 1, 2023
@amydevs amydevs changed the title Remove Implement timeoutMiddleware and remove metadata.authorization for src/nodes/agent Domain Implement timeoutMiddleware and Remove metadata.authorization for src/nodes/agent Domain Nov 1, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
development Standard development
2 participants