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

Have a default config file. #50

Open
Inheritanc-e opened this issue Dec 31, 2020 · 38 comments
Open

Have a default config file. #50

Inheritanc-e opened this issue Dec 31, 2020 · 38 comments
Labels
area: backend status: discussion Discussing the implementation type: enhancement Changes or improvements to existing features

Comments

@Inheritanc-e
Copy link

We should have a default config file so that the people that want to contribute don't have to constantly switch between their editor/IDE to discord just to get the id's of the channels or roles. The information in the config file would be accessible across the entire project. The config file could either be json or yaml. We would initialize the information stored in the config in constants.py, and we would import those information across the project.

@Shivansh-007
Copy link
Contributor

Well this is just a idea, when the bot starts in a particular server it will create a config.json or config.py file in the root directory containing the emojis and its ids, the roles and its ids and same for the channels. And we could just copy paste these in our python code.

@Inheritanc-e
Copy link
Author

What do you mean?
The bot should be configured in such a way that it would fulfill the needs of the Gurkult server.

@Vyvy-vi
Copy link

Vyvy-vi commented Dec 31, 2020

we could have the file generate when the bot is run(dynamic), or should we add it to the repo(static)?

@Inheritanc-e
Copy link
Author

Why would we want to generate a file?
We should just have a default config file on the repo, which would be initialized in constants.py.

@Shivansh-007
Copy link
Contributor

To avoid the work of getting all ids of roles/emojis/channels

@Shivansh-007
Copy link
Contributor

we could have the file generate when the bot is run(dynamic), or should we add it to the repo(static)?

Dynamically when bot is run.

@Inheritanc-e
Copy link
Author

Inheritanc-e commented Dec 31, 2020

Why though?
The bot is configured for the Gurkult server and the default-config would just be the one in the repo.
The bot would interact with config.json which could be constantly updated.

@Vyvy-vi
Copy link

Vyvy-vi commented Dec 31, 2020

That could also be done...
We have 1 default json config which updates when bot is run 👍
(I was talking of generation, since ppl might use this in other servers)

@Shivansh-007
Copy link
Contributor

Yeah just like vyvy said

@Inheritanc-e
Copy link
Author

Are u saying that we create such a configuration that when people would run the bot in their servers that it would get the required channel and role ids and replace the once in config-default.json.
And for the bot in the Gurkult server, the config file would be constantly be updated.

@Shivansh-007
Copy link
Contributor

Yes

@Inheritanc-e
Copy link
Author

Inheritanc-e commented Dec 31, 2020

I guess we can do that. Let's see what others have to say.

@gustavwilliam
Copy link
Member

gustavwilliam commented Jan 2, 2021

I like the idea of having some default config. Maybe we could include a file that defaults to the bot test server, so it's easy to get started with contributing.

Regarding scalability, I think we should have a setup wizard command for servers in the future, but that's nothing we should think about right now imo. For now, I think a config file would be fine, that you duplicate (and change the name of to something predefined, like constants.py) and put your own values in. When the bot start, it first looks for constants.py and if it doesn't find it, it defaults to default-constants.py. (It doesn't have to be a python file. It would probably be YAML).

@gustavwilliam gustavwilliam added area: backend status: discussion Discussing the implementation type: enhancement Changes or improvements to existing features labels Jan 2, 2021
@Inheritanc-e
Copy link
Author

Inheritanc-e commented Jan 2, 2021

Ye, but we could also make it so that they wouldn't have to go through the hassle of getting the ids of channels and roles.
The bot could just get it for them and create the config.yaml or config.json file (I prefer json as it is very similar to python's list and dictionary and I think more people would be familiar with json rather than yaml) . This should also only work on servers that have the same channels and roles as the test server, and for the actual bot we should have a config file that would be updated when some changes made in the server, an example of this would be a channel being deleted.
We wouldn't want things in the config file that take up unnecessary space.

@gustavwilliam
Copy link
Member

That would require having the exact same channel names?

@Inheritanc-e
Copy link
Author

Inheritanc-e commented Jan 2, 2021

Yes it would, as it would indicate that they are configuring the bot to contribute to Gurkult.

@gustavwilliam
Copy link
Member

Why wouldn't they just join the server and the test server? If they really want to do it on their own, they could possibly just enter the IDs imo.

@Inheritanc-e
Copy link
Author

Inheritanc-e commented Jan 2, 2021

By having this, the people that don't want to test in our test server, wouldn't have to go through the hassle of getting the ids of all the channels and roles. I mean not everyone would want to test in the test server.
This would create a config.(json or yaml) file and then this file would be updated according to the changes made to the server.

@Vyvy-vi
Copy link

Vyvy-vi commented Jan 2, 2021

perhaps we could make a setup.py file, to make the config.json file(it can be run, when using the bot for the first time)
imho, keeping it customisable according to contributors/users(since, this is oss), for testing in their own servers, if they want..
Moreover, if we don't make a config file or something like that, we might have to add extra guidelines in the readme on how to change the values etc...

@Shivansh-007
Copy link
Contributor

Well that is something I was working in, and it is a time taking process and Python Discord is already building a smart config, so either we what till. March for that or you gives me sometime to come with a way to do that.

@Inheritanc-e
Copy link
Author

Inheritanc-e commented Jan 3, 2021

so either we what till. March for that.

Why would we have to wait?
We could just make a config that suits our needs rather.
The only thing that this config is supposed to do is store the ids of roles and channels in config.(YAML or JSON).

@vivekashok1221
Copy link
Member

perhaps we could make a setup.py file, to make the config.json file(it can be run, when using the bot for the first time)
imho, keeping it customisable according to contributors/users(since, this is oss), for testing in their own servers, if they want..
Moreover, if we don't make a config file or something like that, we might have to add extra guidelines in the readme on how to change the values etc...

I like the general idea but I'm unsure as to whether we should wait for Smart Config to be developed or make something of our own in the mean time.

Just a small note, we shouldn't call it setup.py since that's meant for metadata about the project and for pip to manage dependencies, etc. . It's never directly run, too.
We can make a .py script of an appropriate name in /scripts. I have consulted with people and running the script with pipenv seems to be a clean option.

@Inheritanc-e
Copy link
Author

Inheritanc-e commented Jan 3, 2021

Right, but that's not the purpose of this issue though.
For this issue, my suggestions are:

  • Have a default-config.(JSON or YAML) file.
  • The bot will get the ids of roles and channels and create a new file called config. (JSON or YAML) with all the retrieved information.
  • When there are changes made in the server such as creating a role, channel, etc, or deleting it, the bot would update the config. (JSON or YAML) file.
  • For constants.py we should use collections.nameduple to access the information in config. (JSON or YAML).

@gustavwilliam
Copy link
Member

gustavwilliam commented Jan 3, 2021

For testing purposes, this could possibly work great. We could probably make sure that the user just runs it through enabling a DEV flag or something. This means we could add something to automatically create categories and roles that don't yet exist, through checking which ones already exist in the server. This would allow the bot to update the server to match the default config easily, on first startup and after pulling new versions, which would be quite fantastic for a testing server (instead of letting the user create roles themselves, like in your third bullet point).

@Inheritanc-e
Copy link
Author

Inheritanc-e commented Jan 3, 2021

I think you misunderstood my third bullet point.
What I meant was when the bot would be running on the server, we wouldn't want to have roles and channels that are already deleted or have been edited, so we should listen for those events through the bot and the bot would change the information in config. (JSON or YAML).

Also I agree with your idea on creating the roles and channels in the server when first ran,

@gustavwilliam
Copy link
Member

So, the user could have a test server that doesn't have all of the required roles?

@Inheritanc-e
Copy link
Author

Inheritanc-e commented Jan 3, 2021

Well we could create the roles that are missing as u suggested.

@gustavwilliam
Copy link
Member

This is what I'm proposing that the bot does when the script is run (could be configured to run automatically on every git pull)

  1. Check what roles and channels exist
  2. Update IDs in config file to match the ones on the server
  3. Create the ones that don't exist and store IDs

The user could also choose to run it manually whenever they think it's needed, using something like pipenv run sync-ids.

@Inheritanc-e
Copy link
Author

Why would we want to update it only when ran?
Why not immediately after the changes made?

@gustavwilliam
Copy link
Member

Well, here are the times that I think we should run it:

  • On git pull, if configured to do so
  • When running it directly through a command

Do we also want it to run every time the bot starts? Nah, that's probably overkill, or what do you think?

@Inheritanc-e
Copy link
Author

Run what may I ask

@gustavwilliam
Copy link
Member

The script that updates the config file to match the server.

@Inheritanc-e
Copy link
Author

Inheritanc-e commented Jan 3, 2021

It wouldn't be a script, it would just be a cog that listens for events in the server and update the config file accordingly.
Which would be located in bot/exts/backend/ under the name of updater.py

But, we should also do what you suggested and create the required channels and roles if they do not exist, in the backend folder.

@Inheritanc-e
Copy link
Author

Inheritanc-e commented Jan 10, 2021

So here is the plan.

  1. We would have a syncer.py in the backends folder which would sync the changes made in the server to the config file.

  2. We would have a check_required_keys function in our constaints.py file which would check if the required keys exist or not.

  3. We would use the check_required_keys function in the syncer.py file to check if the server has the needed roles and channels. When there are changes made in the server we would check if those changes were made to the required keys and if they were then we would try to recreate those keys if we can't then we would dm the server owner asking them to create the required roles and channels.

  4. If all the required channels and ids exist then we would create a new file called config.(JSON or YAML), and store all information there.

@gustavwilliam
Copy link
Member

gustavwilliam commented Jan 10, 2021

Why do you want a sync that goes the SERVER => CONFIG direction? If people create a bunch of roles, they should not be added automatically to the config files. Instead, the user should add it manually, if they make changes.

Especially in test servers, there are often sudo or testers roles, made to let people test freely on the server. If all roles were added to the config automatically, this would be a total mess.

For the record, I've already described how this would preferably work in #50 (comment)

@Inheritanc-e
Copy link
Author

Ye that does make sense.

So..., we would have a script that creates a config file containing all the required information in the server, if some of the required information is not present in the server then the bot would create it, and add it to the config file?

Also, when would the bot check if the roles and ids exist or they have been deleted?

@gustavwilliam
Copy link
Member

Basically, yeah. These are the two files that I think should exist, where both should have either a JSON or YAML format. The names can be changed.

  • server-config — auto-generated config file with the correct IDs from the server.
  • server-config-list — the parts of the server config that should be present in the server-config file. This is a bit like "requirements.txt", where we list the things that should exist in the server, including channels, roles, Webhooks and whatever else we want to include. All items in this file should have an attribute for if it's required or not. For example, you may not require an advent of code channel, while you require a devlog

In order to create server-config, we need a script. This would be a python script that has a few flags available, though these could be added as we go along, if we want to. We should also be able to set this up to run on at least startup, pulls and to be run manually from the command line:

  • auto-config.py

This script would be a bit more complex, but the goal of it is to have valid IDs in server-config, synced from the names of the channels that server-config-list specifies should exist.

@Inheritanc-e
Copy link
Author

Sounds good. Now we need to agree on the type of file that needs to be used.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area: backend status: discussion Discussing the implementation type: enhancement Changes or improvements to existing features
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants