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

Improve agent configuration #486

Closed
jakubkoci opened this issue Oct 13, 2021 · 6 comments
Closed

Improve agent configuration #486

jakubkoci opened this issue Oct 13, 2021 · 6 comments

Comments

@jakubkoci
Copy link
Contributor

jakubkoci commented Oct 13, 2021

As a Developer, I want to have access to config via agent instance, so that I can do that wherever I'm using agent. For example, I can use label when running two agent instances to distinguish between them in logs.

As a Developer, I want to group agent config to make clear how each part relate together and what modules are influenced by a specific group.

const agentConfig = {
  endpoints: process.env.AGENT_ENDPOINTS?.split(','),
  label: process.env.AGENT_LABEL || '',
  wallet: {
    id: process.env.WALLET_NAME || '',
    key: process.env.WALLET_KEY || '',
    publicDidSeed: process.env.PUBLIC_DID_SEED || '',
  },
  connections: {
    autoAcceptConnections: true,
  },
  mediation: {
    autoAcceptMediationRequests: true,
  },
  ledger: {
    genesisPath: '',
    poolName: `pool-name`,
  },
  logger: new ConsoleLogger(LogLevel.debug),
}

This structure is just to demonstrate the idea, I'm not saying it should look exactly like that :)

As a Developer, I want to define a config in JSON file, so that the config will be more explicit and extracted outside of code.

@TimoGlastra
Copy link
Contributor

I agree! I like the nesting of grouped attributes, and think it would be good to be able to access the agent config the agent instance. As it's all getters and read only properties it's not opening the gates for unwanted modifications.

One thing I was thinking about whether we can remove the redundancy from the nested properties. So instead of autoAcceptConnections all it autoAccept as it's under connections key. Same with autoAcceptRequests under mediation key. Or do you think it is valuable to keep it explicit?

@berendsliedrecht
Copy link
Contributor

I think config.connections.autoAccept is explicit enough. Also would it not be useful if the logger could be an array of loggers? That way you could have the console logger for "direct logging" and use a file logger for referencing it later.

@TimoGlastra
Copy link
Contributor

Re the logger, I'd say you need to implement your own logger that internally handles logging to multiple outputs. Most logger libraries have that built in

@berendsliedrecht
Copy link
Contributor

Re the logger, I'd say you need to implement your own logger that internally handles logging to multiple outputs. Most logger libraries have that built in

Ah yeah, that would also work.

@jakubkoci
Copy link
Contributor Author

Yes! I think config.connections.autoAccept is great 👍 I just copy-pasted and missed the opportunity to remove unnecessary words and utilize the context of the parent object :)

@TimoGlastra
Copy link
Contributor

We now have modules that configure different parts of the framework. Please open a new issue if other improvement are needed

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

No branches or pull requests

3 participants