ubiquity / ubiquibot

Putting the 'A' in 'DAO'
https://github.com/marketplace/ubiquibot
MIT License
17 stars 61 forks source link

Separate Partner and Bot Configs #859

Closed rndquu closed 7 months ago

rndquu commented 11 months ago

Depends on https://github.com/ubiquity/ubiquibot/pull/644

Check this js config object. It consists of the following param categories:

  1. Partner config params (ex: price.baseMultiplier, assign.bountyHunterMax, etc...) which are set in the partners' bot yml configs (example)
  2. Bot config params (ex: process.env.SUPABASE_URL, process.env.X25519_PRIVATE_KEY, etc...) which are taken from env variables and are related solely to the bot's running instance (i.e. not to partner projects)

We should separate this js config object in 2 different js configs (partner params and bot env params) so that they don't intermingle.

What should be done:

  1. Separate this config in 2 different js config objects (partner and env configs)
  2. Check that all of the config params are used (chances are that some of the params are not used anywhere so they can be removed)
  3. Update README.md with relevant config param descriptions
rndquu commented 11 months ago

@whilefoo This was originally your idea, pls check the description

Keyrxng commented 11 months ago

As someone who doesn't have an extremely in depth understanding of all of the internals it would be good to have that clarified better because you mentioned process.env.FOLLOW_UP_TIME but I thought that was partner specific until I read that.

Whichever config holds the least properties I assume would be bot config so something like:

module.exports = {
  log: {
    logEnvironment: process.env.LOG_ENVIRONMENT || "production",
    level: (process.env.LOG_LEVEL as LogLevel) || LogLevel.DEBUG,
    retryLimit: Number(process.env.LOG_RETRY) || 0,
  },
  supabase: {
    url: process.env.SUPABASE_URL ?? "",
    key: process.env.SUPABASE_KEY ?? "",
  },
  telegram: {
    token: process.env.TELEGRAM_BOT_TOKEN ?? "",
    delay: process.env.TELEGRAM_BOT_DELAY ? Number(process.env.TELEGRAM_BOT_DELAY) : botDelay,
  },
  logNotification: {
    url: process.env.LOG_WEBHOOK_BOT_URL || "",
    secret: process.env.LOG_WEBHOOK_SECRET || "",
    groupId: Number(process.env.LOG_WEBHOOK_GROUP_ID) || 0,
    topicId: Number(process.env.LOG_WEBHOOK_TOPIC_ID) || 0,
    enabled: true,
  },
};

Since the AI features are optional that would be partner specific? or being an AI powered repo manager would that be a bot level config item?

Also we need a uniform way to read the OpenAi API key as /ask and /review both operate pulling from their specific config types instead of one shared location and similarity reads from process.env.OPENAI_API_KEY.

Also the readme states that the api key should be set in .env but I'm sure I was told to remove reading from .env from within the configs, I may be mistaken but I'm adding process.env.OPENAI_API_KEY for testing then removing it for pushing.

rndquu commented 11 months ago

@Keyrxng

it would be good to have that clarified better because you mentioned process.env.FOLLOW_UP_TIME but I thought that was partner specific

You're right, it is partner specific, my mistake, updated the issue description.

Whichever config holds the least properties I assume would be bot config

If a config param is read from a yml file (example) then it is considered to be a partner config. If a config param is read only from an env variable (example) then it is considered to be the bot's param (i.e. not partner specific param).

Since the AI features are optional that would be partner specific? or being an AI powered repo manager would that be a bot level config item?

Right now we have only 2 AI related config params (openAIKey and openAITokenLimit) both of which are partner specific.

Also we need a uniform way to read the OpenAi API key as /ask and /review both operate pulling from their specific config types instead of one shared location and similarity reads from process.env.OPENAI_API_KEY.

I think that partner specific openai API params should be used both for /ask and /review commands (if this answers your question).

Also the readme states that the api key should be set in .env but I'm sure I was told to remove reading from .env from within the configs, I may be mistaken but I'm adding process.env.OPENAI_API_KEY for testing then removing it for pushing.

process.env.OPENAI_API_KEY is indeed used here. I think it should be refactored to use openai API key from a partner config. Using a shared openai API key will bring us to rate limits and paid plans. It is simpler to make partners setup openai themselves.

Keyrxng commented 11 months ago

/start

ubiquibot[bot] commented 11 months ago

Too many assigned issues, you have reached your max of 2

rndquu commented 11 months ago

@Keyrxng There is a huge refactor coming so it makes sense to wait for that PR and then solve the current issue

ubiquibot[bot] commented 7 months ago
+ Evaluating results. Please wait...
ubiquibot[bot] commented 7 months ago

@pavlovcik the deadline is at 2024-02-17T14:34:20.960Z

ubiquibot[bot] commented 7 months ago
+ Evaluating results. Please wait...
ubiquibot[bot] commented 7 months ago

[ 186 WXDAI ]

@pavlovcik
Contributions Overview
ViewContributionCountReward
IssueTask1.0075
ReviewComment1355.5
ReviewComment1355.5
Conversation Incentives
CommentFormattingRelevanceReward
This should have merged this in weeks ago. The number of outstan...
9.1-9.1
@0xcodercrane In this case you should consider just regenerating...
3.8-3.8
Don't think it matters the order regarding conflicts but technic...
1.9-1.9
Will need to e2e test and merge...
0.7-0.7
Review comments at https://github.com/ubiquity/ubiquibot/pull/84...
1-1
There are many dimensions of your proposal to consider so it's h...
8.4-8.4
This should work on personal accounts (not just organizations) a...
2.4-2.4
Notes to self on incentive calculation architecture: - work bac...
4.8
li:
  count: 2
  score: "2"
  words: 21
-4.8
I think next week. It feels really close to being done but I'm a...
2.8-2.8
@wannacfuture its better that you branch from here instead of pu...
1.9-1.9
> I'm trying to use the new version with workflow dispatcher and...
8.5
code:
  count: 1
  score: "1"
  words: 26
-8.5
You guys should make sure you're using the same version of Node....
9.2-9.2
@gitcoindev maybe you can help fix the knip CI here...
1-1
This should have merged this in weeks ago. The number of outstan...
9.1-9.1
@0xcodercrane In this case you should consider just regenerating...
3.8-3.8
Don't think it matters the order regarding conflicts but technic...
1.9-1.9
Will need to e2e test and merge...
0.7-0.7
Review comments at https://github.com/ubiquity/ubiquibot/pull/84...
1-1
There are many dimensions of your proposal to consider so it's h...
8.4-8.4
This should work on personal accounts (not just organizations) a...
2.4-2.4
Notes to self on incentive calculation architecture: - work bac...
4.8
li:
  count: 2
  score: "2"
  words: 21
-4.8
I think next week. It feels really close to being done but I'm a...
2.8-2.8
@wannacfuture its better that you branch from here instead of pu...
1.9-1.9
> I'm trying to use the new version with workflow dispatcher and...
8.5
code:
  count: 1
  score: "1"
  words: 26
-8.5
You guys should make sure you're using the same version of Node....
9.2-9.2
@gitcoindev maybe you can help fix the knip CI here...
1-1

[ 0 WXDAI ]

@BeanieMen
Contributions Overview
ViewContributionCountReward
ReviewComment10
Conversation Incentives
CommentFormattingRelevanceReward
🎉...
---

[ 52.6 WXDAI ]

@gitcoindev
Contributions Overview
ViewContributionCountReward
ReviewComment752.6
Conversation Incentives
CommentFormattingRelevanceReward
> There also seems to be an issue with ubiquibot-logger > > `...
9.1
code:
  count: 4
  score: "4"
  words: 4
-9.1
> > Hi @whilefoo , the `ubiquibot-logger` is an ESM module https...
7.3
code:
  count: 5
  score: "5"
  words: 7
-7.3
> > > Hi @whilefoo , the `ubiquibot-logger` is an ESM module htt...
14.9
code:
  count: 7
  score: "7"
  words: 10
-14.9
hi @whilefoo I will give this priority and try to fix till Monda...
1.4-1.4
@whilefoo I am on v0.3.4 already, so far no luck but closer and ...
5.2-5.2
hi @whilefoo could you please check ubiquibot-logger v0.3.5 ? I...
8.9
code:
  count: 1
  score: "1"
  words: 6
-8.9
Btw, about @pavlovcik 's last comment, I agree that we should sy...
5.8-5.8

[ 25.4 WXDAI ]

@Keyrxng
Contributions Overview
ViewContributionCountReward
IssueComment125.4
Conversation Incentives
CommentFormattingRelevanceReward
As someone who doesn't have an extremely in depth understanding ...
25.4
code:
  count: 8
  score: "2"
  words: 15
0.84525.4

[ 1.6 WXDAI ]

@0xcodercrane
Contributions Overview
ViewContributionCountReward
ReviewComment11.6
Conversation Incentives
CommentFormattingRelevanceReward
should I merge https://github.com/ubiquity/ubiquibot/pull/643 in...
1.6-1.6

[ 0.6 WXDAI ]

@wannacfuture
Contributions Overview
ViewContributionCountReward
ReviewComment10.6
Conversation Incentives
CommentFormattingRelevanceReward
should I resolve the conflicts? @pavlovcik ...
0.6-0.6

[ 366 WXDAI ]

@rndquu
Contributions Overview
ViewContributionCountReward
IssueSpecification141.4
IssueComment345.2
ReviewComment3279.4
Conversation Incentives
CommentFormattingRelevanceReward
Depends on https://github.com/ubiquity/ubiquibot/pull/644 Che...
41.4
a:
  count: 4
  score: "4"
  words: 4
li:
  count: 5
  score: "5"
  words: 135
code:
  count: 3
  score: "3"
  words: 14
141.4
@whilefoo This was originally your idea, pls check the descripti...
20.82
@Keyrxng > it would be good to have that clarified better be...
37.8
a:
  count: 5
  score: "5"
  words: 5
code:
  count: 3
  score: "3"
  words: 5
0.9137.8
@Keyrxng There is a [huge refactor](https://github.com/ubiquity/...
5.4
a:
  count: 1
  score: "1"
  words: 2
0.7155.4
@pavlovcik There is the supabase [diff tool](https://supabas...
84.2
a:
  count: 4
  score: "8"
  words: 7
li:
  count: 7
  score: "14"
  words: 110
code:
  count: 10
  score: "20"
  words: 11
-84.2
@pavlovcik Regarding the `locations` and metadata. This i...
192
h2:
  count: 2
  score: "4"
  words: 11
h3:
  count: 4
  score: "8"
  words: 4
a:
  count: 2
  score: "4"
  words: 3
li:
  count: 10
  score: "20"
  words: 170
code:
  count: 21
  score: "42"
  words: 67
td:
  count: 16
  score: "32"
  words: 57
-192
> Alternatively we do the two API calls but there's a good chanc...
3.2-3.2

[ 25.4 WXDAI ]

@whilefoo
Contributions Overview
ViewContributionCountReward
ReviewComment525.4
Conversation Incentives
CommentFormattingRelevanceReward
I'm trying to use the new version with workflow dispatcher and I...
6.8
code:
  count: 1
  score: "1"
  words: 26
-6.8
There also seems to be an issue with ubiquibot-logger ``` co...
7.9
code:
  count: 1
  score: "1"
  words: 0
-7.9
> Hi @whilefoo , the `ubiquibot-logger` is an ESM module https:/...
8.9
code:
  count: 5
  score: "5"
  words: 7
-8.9
> Following the article https://dev.to/tigawanna/building-and-pu...
1.2-1.2
@gitcoindev it's working now, thanks!...
0.6-0.6

[ 3.7 WXDAI ]

@EtherealGlow
Contributions Overview
ViewContributionCountReward
ReviewComment53.7
Conversation Incentives
CommentFormattingRelevanceReward
I pray for the refactor to be done quickly 🙏...
0.9-0.9
Do you guys have any estimate on when the refactor will be done ...
1.3-1.3
Any new estimate for how much time is left?...
0.9-0.9
🙏...
---
When will the refactor be done 😭...
0.6-0.6