Generate SQLAlchemy models based on the current schema, using SQLAcodegen, add PostgreSQL dependencies

[D0rs4n]

I ran the model generation, surprisingly it did not end up generating Table definitions at all. It still lacks of some Validation (However in some places it managed to generate those checks) however some of the checks defined on the Django side still need to be implemented. (Most of them are fairly easy though). Although, I believe opening a Draft Pull Request to discuss further things to do/implement is vital. Originally, everything was in one file, so I took the liberty and split them into multiple files. It had some issues with DeletedMessages and Messages but I believe I managed to resolve this. Closes #16

[jchristgit]

Should we remove the Api prefix from models, and add uppercase letters where reasonable? Such as ApiOfftopicchannelname -> OffTopicChannelName?

I think multiple modules is fine, but we should have an __init__.py in there importing them so we can import the models from a single import.

[D0rs4n]

Yes, sorry. The class names were generated that way, there is a naming convention of some sort. I'll remove the API prefix and add inits. Also, how does the overall picture looks like? It had some problems with the messages module, but I fixed it, I believe.

[D0rs4n]

Should we remove the Api prefix from models, and add uppercase letters where reasonable? Such as ApiOfftopicchannelname -> OffTopicChannelName?

I think multiple modules is fine, but we should have an __init__.py in there importing them so we can import the models from a single import.

I've added the changes mentioned, however I'm not sure I understand why the max line length is 88.

[jchristgit]

Yes, sorry. The class names were generated that way, there is a naming convention of some sort.

No need to be sorry at all 🙂 I realized there was some convention. Thanks for updating it!

Also, how does the overall picture looks like? It had some problems with the messages module, but I fixed it, I believe.

With overall picture, do you mean whether this can un-drafted? From a quick view, it looks good to me, I can give it a thorough review once it's ready to go.

[jchristgit]

Sorry, that was the wrong button....

[D0rs4n]

Yes, sorry. The class names were generated that way, there is a naming convention of some sort.

No need to be sorry at all 🙂 I realized there was some convention. Thanks for updating it!

Of course, it was a bit inconsistent with this prefix.

Also, how does the overall picture looks like? It had some problems with the messages module, but I fixed it, I believe.

With overall picture, do you mean whether this can un-drafted? From a quick view, it looks good to me, I can give it a thorough review once it's ready to go.

There's only one check remaining that has to be implemented, and that is the check in the User model that ensures that the role being added is an existing Role. For that, I have to create a session, and in order to create this session I'll need a database connection string, so I believe we might have to add it to the environment variables. However, after that and testing the checks out, it should be ready to go! 👍

[D0rs4n]

However it's not the scope of this PR, would it mean a problem if I added PostgreSQL to the docker-compose file, and the drivers to the poetry environment? @jchristgit (I have the changes prepared locally, since I was testing the models, but haven't added them to the PR just yet.)

[jchristgit]

However it's not the scope of this PR, would it mean a problem if I added PostgreSQL to the docker-compose file, and the drivers to the poetry environment? @jchristgit

No, not at all, go PostgreSQL!!! 🐘

[D0rs4n]

However it's not the scope of this PR, would it mean a problem if I added PostgreSQL to the docker-compose file, and the drivers to the poetry environment? @jchristgit

No, not at all, go PostgreSQL!!! 🐘

Alright! 👍 Apart from the linting errors(Should we change the max-line-length to 120?) , I believe it's done. I have tested out few of the checks and relationships, they seem to work just fine.

[jchristgit]

This PR looks good to me now. Two small points:

• Could you look into fixing CI for our tests to pass?
• Since we now have the model definition in two projects, it would be great to maybe add some form of reminder to the site pull requests about whether they update the model definitions in a way that requires an update here. I do not think we should put a freeze to model changes on the old site, at all. First off, it's hard to implement, and second we still want to be able to make modifications. @jb3 would it be possible to create a pull request template in the site repository that goes something like "If you have updated the models: [] I have created a corresponding PR to update the models in the API project, [] I have not" or something.

[D0rs4n]

This PR looks good to me now. Two small points:

* Could you look into fixing CI for our tests to pass?

* Since we now have the model definition in two projects, it would be great to maybe add some form of reminder to the site pull requests about whether they update the model definitions in a way that requires an update here. I **do not** think we should put a freeze to model changes on the old site, at all. First off, it's hard to implement, and second we still want to be able to make modifications. @jb3 would it be possible to create a pull request template in the site repository that goes something like "If you have updated the models: [] I have created a corresponding PR to update the models in the API project, [] I have not" or something.

Could you look into fixing CI for our tests to pass?

@jchristgit As for now, it wouldn't make sense to add Postgres to the CI, since we don't have any relating tests. As a possible solution, maybe I could make "database_url" Optional for now? Also, am I also supposed to fix linting as well? I've added a new configuration that is similar to the ones used in other pydis projects, should I PR the new config? (I mean flake config)

[jchristgit]

As for now, it wouldn't make sense to add Postgres to the CI, since we don't have any relating tests. As a possible solution, maybe I could make "database_url" Optional for now?

Could we get away with setting that to some bogus value for now? I don't think making it optional makes sense from a deployment nor testing perspective. 99% of what the API does needs the database.

Also, am I also supposed to fix linting as well? I've added a new configuration that is similar to the ones used in other pydis projects, should I PR the new config? (I mean flake config)

Yes, feel free to add it into this PR.

[D0rs4n]

As for now, it wouldn't make sense to add Postgres to the CI, since we don't have any relating tests. As a possible solution, maybe I could make "database_url" Optional for now?

Could we get away with setting that to some bogus value for now? I don't think making it optional makes sense from a deployment nor testing perspective. 99% of what the API does needs the database.

Also, am I also supposed to fix linting as well? I've added a new configuration that is similar to the ones used in other pydis projects, should I PR the new config? (I mean flake config)

Yes, feel free to add it into this PR.

I've already added, and based on that config the linting should pass. (Update: it did :D ) @jchristgit