Added postgre to database_uri

[mtreacy002]

Description

Include a summary of the change and relevant motivation/context. List any dependencies that are required for this change.

Fixes #514

Type of Change:

• Code

Code/Quality Assurance Only

• New feature (non-breaking change which adds functionality pre-approved by mentors)

How Has This Been Tested?

Running python run.py works as usual

Checklist:

• My PR follows the style guidelines of this project
• I have performed a self-review of my own code or materials
• I have commented my code or provided relevant documentation, particularly in hard-to-understand areas

Code/Quality Assurance Only

• My changes generate no new warnings

[mtreacy002]

Hi @isabelcosta or @ramitsawhney27. When you have time, can you please check this pull request? Thanks. 😉

[vkWeb]

@mtreacy002 Hi :) I am new to the mentorship-backend so take my advice with a grain of salt.

In my projects, I handle multiple db options as follows:

SQLALCHEMY_DATABASE_URI = os.getenv("DB_URI") or "sqlite:///local_data.db"

This allows the user / contributor to input any database they want to use via a environment variable. And in case they don't provide any env var, the db connects to the default sqlite database.

[mtreacy002]

@mtreacy002 Hi :) I am new to the mentorship-backend so take my advice with a grain of salt.

In my projects, I handle multiple db options as follows:

SQLALCHEMY_DATABASE_URI = os.getenv("DB_URI") or "sqlite:///local_data.db"

This allows the user / contributor to input any database they want to use via a environment variable. And in case they don't provide any env var, the db connects to the default sqlite database.

That's sounds ideal, @vkWeb, but should we also apply it here in Mentorship_System backend code, so, change the database reference in LocalConfig and TestingConfig of the config.py to use your line of code instead and change .env to include the DATABASE_URI of our preference. This way, no need to have separate db type commented out/uncomment every time user wants to use their favourite local db.
@isabelcosta, what do you think about this?

[vkWeb]

@mtreacy002 I'm glad you liked the idea 😄. I think we should simply change the BaseConfig class. All other classes seems to inherit BaseConfig.

Also, shouldn't we simply allow DB_URI as env var instead of building one using all the db credentials? The contributor can simple provide the DB_URI in .env.

[mtreacy002]

@vkWeb, I think the reason why the maintainers of mentorship-system put DB_URI in BaseConfig as it is now is to make it easier to control which remote db to be used as the primary db for Development, Staging and Production. It's pretty sensible to limit contributors option to only in the Local and Testing config. The only way us as contributors could choose to switch to our db preference is then to set FLAS_ENVIRONMENT_CONFIG to 'local'.
IMO it's best to keep the changes you suggested to just inside these Local and Testing and leave the other config as it is.
What do you think? We should also ask @isabelcosta opinion on this... 😃

[mtreacy002]

This is also then allow contributors who don't want to use their own local server to use which ever primary db already set in the Development branch's server/s (atm in Heroku, before w elasticbeanstalk). These contributors don't need to provide any credentials if they want to use just the remote server/db provided by mentorship-backend. ...mmm.. I'm pretty sure they both (heroku and elasticbeanstalk servers) using postgre. Or maybe not...😂 do you know what our remote db is, @vkWeb?

[vkWeb]

@vkWeb, I think the reason why the maintainers of mentorship-system put DB_URI in BaseConfig as it is now is to make it easier to control which remote db to be used as the primary db for Development, Staging and Production.

@mtreacy002 actually, I was saying that currently we are using a function build_db_uri to build db_uri from user's env vars. IMO this whole function can be removed and we can use a simple DB_URI env var which will point to whichever remote / local db user wants. The URI will be built by the user.

It's pretty sensible to limit contributors option to only in the Local and Testing config. The only way us as contributors could choose to switch to our db preference is then to set FLAS_ENVIRONMENT_CONFIG to 'local'.
IMO it's best to keep the changes you suggested to just inside these Local and Testing and leave the other config as it is.

See, here's what I propose: we put that or statement in BaseConfig, remove the build_db_uri & all the related env vars if you and @isabelcosta think it make sense. Then we can overwrite the SQLALCHEMY_DB_URI in Testing, Local, Development. Does that makes sense @mtreacy002?

do you know what our remote db is, @vkWeb?

API is hosted on heroku, so maybe heroku postgres 😛. I don't know, it's just a guess.

[mtreacy002]

Yep, sounds good to me... We'll wait and see what @isabelcosta think, I suppose. Maybe once she's not too busy with proposals selection. 😉

[devkapilbansal]

@isabelcosta needs your input here

[epicadk]

I don't think this is the way to go. I think we should use environment variables for this.

[mtreacy002]

@epicadk, @devkapilbansal and @vj-codes , I'll close this issue as this is a very old issue and the postgres db_uri is covered within the more recent issue (pr #1029)