Add an Auto Config system and have a default config file. #73
Conversation
…he config list and the default config files for the bot, modified constants.py to use the config files to store channel and role ids. Used collections.namedtuple for channel and role constants. Modified return statments in __init__.py. Synced the project to use modified constants.py
|
|
||
| Polls: | ||
| optional: False | ||
|
|
There was a problem hiding this comment.
Add the un-optional channels here too, and when the bot goes over them, it asks the user whether they want to create the un-optional channels or not.
Same for the roles.
There was a problem hiding this comment.
It is pretty obvious that the channels and roles that have not been marked as optional are mandatory.
There was a problem hiding this comment.
But, to test the github category we need the dev categories, so hence we need them.
There was a problem hiding this comment.
We need them hence it's set to false.
There was a problem hiding this comment.
And if we require them to be mandatory then we could just completely remove them from the config-list.yaml
There was a problem hiding this comment.
We need them hence it's set to false.
the dev-gurkbot, etc. are required for github category and I don't see them here.
There was a problem hiding this comment.
Well they aren't needed for the bot as of now. So....
| class Channels(NamedTuple): | ||
| devalerts = int(os.getenv("CHANNEL_DEVALERTS", 796695123177766982)) | ||
| devlog = int(os.getenv("CHANNEL_DEVLOG", 789431367167377448)) | ||
| class Gurkbot: |
There was a problem hiding this comment.
| class Gurkbot: | |
| class Client: |
There was a problem hiding this comment.
Having the class as the bot's name would make more sense IMO.
There was a problem hiding this comment.
What if the name of the bot changes in future?
There was a problem hiding this comment.
Then we change the name of the class.
There was a problem hiding this comment.
Do we really need this class? Is it related to how auto-config works?
Gurkbot doesn't feel like an apt name.
There was a problem hiding this comment.
I mean would it not make sense to have the tokens and prefix of the bot in a single class?
There was a problem hiding this comment.
Yeah, it would keep the client constants together
There was a problem hiding this comment.
Client can mean many things.
| ERROR_REPLIES = bot_replies["ERROR_REPLIES"] | ||
| POSITIVE_REPLIES = bot_replies["POSITIVE_REPLIES"] | ||
| NEGATIVE_REPLIES = bot_replies["NEGATIVE_REPLIES"] | ||
| ERROR_REPLIES = [ |
There was a problem hiding this comment.
Why did u shift the error replies to constants.py from the yaml file?
See fisher's comments on Error Handler Pr
| token = os.getenv('TOKEN') | ||
|
|
||
|
|
||
| Channels = namedtuple( |
There was a problem hiding this comment.
Using a class for this looks better and more readable and easier to understand.
There was a problem hiding this comment.
If we use a class then we would have to insert them again. But we shouldn't have to do that because it already exists in the config, file.
There was a problem hiding this comment.
Instead of using namedtuple, why not pick them like we did for the environment variables?
There was a problem hiding this comment.
What do you mean?
Namedtuple is perfect for storing constants.
There was a problem hiding this comment.
Not sure about this one, chief.
We should discuss this on the server. Readability is lost.
Btw, you can use typing.NamedTuple to create a named tuple and I feel that'd produce more readable code.
There was a problem hiding this comment.
Yeah, I agree with @codephile1221, the readability is lost, @gustavwilliam whats your say on this?
There was a problem hiding this comment.
TBH, readability doesn't matter much since you can look at the original config file.
There was a problem hiding this comment.
How is readability lost?
This is very much understandable, the first parameter of named tuples are keys and the default keyword takes a tuple which are the values for those keys going from left to right.
There was a problem hiding this comment.
Which is easier to understand, the former way or the yours?
There was a problem hiding this comment.
Mine is very much understandable, if you wish to see the ids then the config file exists for that, and you would not understand mine only if you don't know how namedtuple works.
| """ Check's if the guild exists or not, if not then raises NotFound.""" | ||
|
|
||
| if not self.bot.get_guild(int(os.getenv('Guild_ID'))): |
There was a problem hiding this comment.
| """ Check's if the guild exists or not, if not then raises NotFound.""" | |
| if not self.bot.get_guild(int(os.getenv('Guild_ID'))): | |
| """Check's if the guild exists or not, if not then raises NotFound.""" | |
| if not self.bot.get_guild(int(os.getenv('Guild_ID'))): |
There shouldn't be a empty space in def functions.
| return | ||
|
|
||
| else: | ||
| logger.info('Successfully created all the roles and roles') |
There was a problem hiding this comment.
| logger.info('Successfully created all the roles and roles') | |
| logger.info('Successfully created all the roles') |
| for c in self.required()['Channels']: | ||
| await guild.create_text_channel(name=c) |
There was a problem hiding this comment.
| for c in self.required()['Channels']: | |
| await guild.create_text_channel(name=c) | |
| for channel in self.required()['Channels']: | |
| await guild.create_text_channel(name=channel) |
| return | ||
|
|
||
| else: | ||
| logger.info('Successfully created all the roles and channels!') |
There was a problem hiding this comment.
| logger.info('Successfully created all the roles and channels!') | |
| logger.info('Successfully created all the roles.') |
Isn't this only making the channels?
There was a problem hiding this comment.
If the code reached up the else statement then it means that the server doesn't have the channels configured nor does it have the roles configured.
| for c in self.required()['Channels']: | ||
| await guild.create_text_channel(name=c) | ||
|
|
||
| for r in self.check()['Roles']: | ||
| await guild.create_role(name=r) |
There was a problem hiding this comment.
| for c in self.required()['Channels']: | |
| await guild.create_text_channel(name=c) | |
| for r in self.check()['Roles']: | |
| await guild.create_role(name=r) | |
| for channel in self.required()['Channels']: | |
| await guild.create_text_channel(name=channel) | |
| for role in self.check()['Roles']: | |
| await guild.create_role(name=role) |
| 'The bot does not have the required permissions to create the roles and the channels!') | ||
|
|
||
| else: | ||
| logger.info('Successfully created all the roles and channels and roles!') |
There was a problem hiding this comment.
| logger.info('Successfully created all the roles and channels and roles!') | |
| logger.info('Successfully created all the roles and channels!') |
|
@Inheritanc-e Can you fix the conflicts in here? |
|
is this PR still WIP? |
@RohanJnr definitely doesn't seem so |
|
@Inheritanc-e Thanks for all the work you did with this. Sadly it doesn't seem like it'll be finished, so I'm closing the PR. The code is pretty outdated compared to the rest of the bot today, but if you would like to resume something like this in the future, feel free to ping me. |
Fixes #50
Created an auto-config system, added a config folder which contains the config list and the default config files for the bot, modified
constants.pyto use the config files to store channel and role ids. Used collections.namedtuple for channel and role constants. Modified return statements in__init__.py.Synced the project to use modifiedconstants.py