ubiquity / ubiquibot-logger

0 stars 5 forks source link

Decouple GitHub comments #2

Closed 0x4007 closed 2 months ago

0x4007 commented 9 months ago

https://github.com/ubiquity/ubiquibot-logger/pull/1#pullrequestreview-1773488754

gentlementlegen commented 3 months ago

This issue is pretty relevant now that we have plugin that will be probably reusing this a lot. Would help solving comment https://github.com/ubiquibot/command-query-user/pull/1#discussion_r1616425390

Keyrxng commented 3 months ago

@gentlementlegen can we clarify the spec for this a bit?

afaik it should be made possible for it to be a Supabase-only logger but also have the ability to post and edit comments, is that right?

I've decoupled the logger from comments in rpc-handler, I'm assuming it's to be pretty close to this but with settings for the above abilities?

gentlementlegen commented 3 months ago

@Keyrxng We should think more about it but I think the problems we want to solve are:

I don't think it is a good idea to tie Supabase nor GitHub into it as we don't necessarily want to post or save to database, but rather simply log stuff. They can be present but not required.

Keyrxng commented 3 months ago

I understand now, it should be just a logger for whatever env the plugin is in worker or action.

So it should follow the same info > error... api and further log posting/storage is not strictly required.

So either those posting/storage extensions could be removed to streamline things or they should be configurable but disabled by default.

Is it possible for anyone to send logs to SB? Say new plugin devs use the prod db instance in development as-is the way to do things up till now, would they be able to spam the logs? If so it might be a good idea to not include these extensions

gentlementlegen commented 3 months ago

I think it would be nice if each plugin would keep its own logs as it would be quite easier to sort and find the ones we want. Because indeed they could clutter the main db with their own logs.

Keyrxng commented 3 months ago

So ideally the kernel would be handling all of it's errors and being able to log them to SB but the plugin logger probably shouldn't have that feature available? instead should be debugged from it's specific run or worker logs.

The logger could be made to return the log output and it can be used by the invoker to post/store if they want to?


In this sense it's avoiding duplicate code for each plugin and still has nice formatting for comments easily should the plugin decide that's what it wants to do with it's logs, the plugin has it's own adapters and octokit (if it requires) so I think that makes sense because it doesn't make it so easy to have the prod db spammed with logs

ubiquibot[bot] commented 2 months ago
! No price label has been set. Skipping permit generation.
ubiquity-os-testing[bot] commented 2 months ago
! No price label has been set. Skipping permit generation.