Skip to content

Conversation

@edsun123
Copy link
Contributor

For others to look at and add upon.

  1. To test: create heroku app first
  2. save the heroku config var DATABASE_URL in env variable (done automatically in script)
  3. need to save vault password into heroku config var (will not be prompted when running on heroku)

@dwreeves
Copy link
Collaborator

dwreeves commented Sep 26, 2020

Is there any special significance to the env var name DATABASE_URL? I.e. is it a reserved env variable name, or does Heroku do something special with this particular env variable that it wouldn't be able to do with other env variables? I'm not seeing any evidence in Heroku's documentation that there is special significance to it.

Whether or not it is a special env var matters for reasons I'll discuss once we're 100% certain that there is no special significance. If there is however special significance to it then disregard that.

@edsun123
Copy link
Contributor Author

env var name DATABASE_URL doesn't hold any significance. In local machine, the env variable can be anything. However, to clarify, the config var for postgres URI for heroku will always be DATABASE_URL. The reason why they may match is to maintain consistency , but you can change it to your convenience.

@dwreeves
Copy link
Collaborator

dwreeves commented Sep 28, 2020

OK I see what's happening, Heroku auto-populates an env variable called DATABASE_URL after you add the Postgres add-on to an app instance. That's what I meant by special significance, sorry for being unclear.

The reason the special significance matters is because we have a system already for the environment variables prefixed POSTGRES_*, but clearly Heroku does something slightly different. I looked into it a bit more, and I learned that there is a concept in Heroku called heroku stack, and you can set it to be either heroku stack:set heroku-18 and another way that's like heroku stack:set container. The container system takes two different approaches for deployment, the container registry or the "manifest" approach. (Container registry == upload a Docker container's artifact; manifest method == define a file called heroku.yml that's akin to a docker-compose.yml file.) Right now we are doing heroku-18, which avoids explicit containerization on our part. That's the approach we want to do though.

Very boring stuff in the above paragraph. So why's that all matter? The reason is because I was taking the POSTGRES_* config settings style from an open-source project here: https://github.com/cburmeister/flask-bones I figured if it works for them then it should work for us. However, they're deploying using the container registry, which we won't be doing. (At most we could do the manifest thing, but I don't think we should. We can't use the container registry because it requires having Docker on your local machine, which is impossible for Windows Home edition and is therefore a non-starter.) TLDR this means the previous config style of POSTGRES_* vars doesn't work for us, as we are stuck with what Heroku gives us directly (i.e. DATABASE_URL).

I think your solution to it is fine for now, although in the future we may want to consider consolidating everything a bit since right now the config can have values in it that seem contradictory, e.g. POSTGRES_DBNAME says flagging but the dbname was auto-provided in the URL that Heroku gives you. You don't need to worry about that though, it's a vestige of the system I copied from flask-bones and we'll just figure out a more coherent system in time.


Also, I should have clarified that part of the requirements for the issue would be to clarify any steps like adding add-ons etc. but I figured it out. (if you think I'm bad at Heroku for not figuring out that first step immediately, the CRWA will also struggle with it...) Later we will have to add the thing about which add-ons to attach as part of our deployment documentation.


I have your code "working" in the sense that the database is created on Heroku, but right now the database is blank for me. Are you experiencing the same thing there? If so, how do you suppose we initiate the database?

One idea I have is to create a function called database_is_empty(db) that runs really quickly to check to see if the database is empty, and then we can do the following in the create_app() function:

# contents of flagging_site/app.py

def create_app( ... ) -> Flask:

    # ...

    from .data.database import database_is_empty
    if database_is_empty(db):
        from .data.database import init_db
        init_db()

(We don't want to re-init the database each time the app fires up because due to how Heroku works, it ends up running the create_app() function after every snooze. Which means the longer it takes for create_app() to run, the longer the website takes to load after 30 minutes of inactivity.)

(That said, if possible and feasible, I would slightly prefer a solution that initiates the database as part of deployment and not as part of firing up the app instance in create_app. That might involve using the Heroku CLI somehow, or using the Procfile. Not sure.)

@edsun123
Copy link
Contributor Author

Ok sure. I will take a look into automating the init-db so CRWA don't need to go to heroku cli. I will look into saving the password into heroku config vars so that they don't need to remember the password nor exporting the password into heroku config vars.

@dwreeves
Copy link
Collaborator

To clarify, we just need a process to set up and populate the database's tables at all using init_db() in some capacity, whether it's run in the code itself or via the CLI. If it involves using the Heroku CLI then that's fine. It just needs to be easy, replicable, robust, and it needs to not run more than once (or at least not run synchronously with GET requests to the website) so that we don't slow down the website.

Also, that method of running init_db() in Heroku needs to be documented somewhere. Even if it's just a comment here in the PR on how you were able to run init_db() in Heroku, then that's fine.

Were you able to get the database populated? For me following your directions a database existed, but it was blank (no tables whatsoever) because init_db() was not run in the live version (which means in turn schema.sql and the boathouses query were not run.) If your version of the integration server had the populated database, can you clarify how you did it?


There are no issues related to any passwords that need additional work in this issue. The DATABASE_URL contains the password. The URL schema for any Postgres database is postgres://user:pass@host:port/db. So long as Heroku gives you a valid database URL-- and it does-- then this is not a concern. The main concern I was conveying was that the POSTGRES_* values in the Config are not used and may contradict with the DATABASE_URL, which is overall a minor concern but it does make the code more confusing. But I would not focus on that.

@edsun123
Copy link
Contributor Author

edsun123 commented Sep 30, 2020

I need to check again about populating the database in the most recent version of the code. I would also add to documentation about what I have done as well once I check.

@dwreeves
Copy link
Collaborator

dwreeves commented Oct 3, 2020

Never mind on the last part of this request, I found out a pretty decent system for getting the database up and running!

@dwreeves dwreeves merged commit 91b5732 into codeforboston:dev_postgres Oct 3, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants