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

Add Postgres + Sequelize #79

Merged
merged 16 commits into from
Nov 23, 2019
Merged

Add Postgres + Sequelize #79

merged 16 commits into from
Nov 23, 2019

Conversation

gamburgm
Copy link
Member

Long-term goal: add a database to back up our data, that'll let us have a stable data store for our entries (Courses, Professors, etc) and lets us more easily serve that data (and centralize our data) when we can.

This PR just sets up the tables and schema we're going to be using. Getting this out quickly so that our Graduate team can start using it to get data from our new Major table.

Copy link
Member

@dajinchu dajinchu left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good for getting the model set up. Do we have a plan for writing tests? Makes sense for none of this to have tests, but eventually we'll want tests that will hit postgres. What's the game plan for that?
Also what's the game plan for connecting elasticsearch? Either way, in the interest of getting this through so that Graduate isn't blocked, I'd give my 👍

sequelize = new Sequelize(config.database, config.username, config.password, config);
}

fs
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is this importing everything in the folder or something? Is this normal sequelize stuff? I feel weird about this kind of thing

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This was auto-generated by Sequelize, the idea is that you can import db anywhere and fetch each model from there—in other words, yeah.

Copy link
Member Author

@gamburgm gamburgm left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good for getting the model set up. Do we have a plan for writing tests? Makes sense for none of this to have tests, but eventually we'll want tests that will hit postgres. What's the game plan for that?
Also what's the game plan for connecting elasticsearch? Either way, in the interest of getting this through so that Graduate isn't blocked, I'd give my 👍

Fortunately, Sequelize comes with configuration for a test DB out of the box, so we'll be able to hit that however we want.

Next step is to import all the JSON from the scrapers and then convert the IDs into Postgres hits, goal is to get that done by Thanksgiving.

@ryanhugh
Copy link
Collaborator

If y'all are keeping track of historical data you could think about these ideas too:

ryanhugh#53
ryanhugh#50

🙂

@ryanhugh
Copy link
Collaborator

so that our Graduate team can start using it to get data from our new Major table.

Btw, what are the dets on the new Major table? Where do you guys plan on getting info on Majors and Minors (or do you just plan to manually type them in?). Just curious 😛

@gamburgm
Copy link
Member Author

so that our Graduate team can start using it to get data from our new Major table.

Btw, what are the dets on the new Major table? Where do you guys plan on getting info on Majors and Minors (or do you just plan to manually type them in?). Just curious 😛

We got some starting data from the Graduate team that we're going to use for now, but going forward we're hoping that either the Graduate team can insert it themselves or there's a more automated process to do it.

@gamburgm gamburgm merged commit 67954f4 into master Nov 23, 2019
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.

3 participants