Skip to content

Conversation

@mathcolo
Copy link
Contributor

@mathcolo mathcolo commented Apr 17, 2022

Almost works except that DNS lookup in the NTT container cannot find the postgres container for some reason!

It's probably something stupid I'm missing

help

@nathan-weinberg
Copy link
Member

@mathcolo Let's throw some README updates in as well for the people (it's me, I'm the people)

@mathcolo mathcolo changed the title Containerize NTT but it doesn't actually work yet nathan help Containerize NTT Jun 28, 2022
@mathcolo
Copy link
Contributor Author

I think this is almost good to go! @Joshhw we just need to hook up the host's MBTA API key (ideally). I can cover that unless you beat me to it

@Joshhw Joshhw requested a review from thiteixeira July 2, 2022 21:30
@Joshhw
Copy link
Contributor

Joshhw commented Jul 2, 2022

this should be R4R. I need some folks to test it out.

@thiteixeira
Copy link
Contributor

thiteixeira commented Jul 21, 2022

I was playing around with the [stats-page](https://github.com/transitmatters/new-train-tracker/tree/stats-page) branch and tested this one.

Steps (on latest commit - 8ca1522):

  1. docker system prune -a
  2. docker compose up
  3. Fails to connect to the database:
    psycopg2.OperationalError: could not connect to server: Connection refused

Log:
Screen Shot 2022-07-20 at 9 36 49 PM

** Edit **
Forgot to mention that I'm on MacOS M1

@mathcolo
Copy link
Contributor Author

@Joshhw Do you want to address @thiteixeira feedback or me? I know I originally filed this, but I don't want to steal your thunder.

Co-authored-by: Thiago Teixeira <thiteixeira@gmail.com>
@thiteixeira
Copy link
Contributor

I'm happy to give this PR a try as well

@Joshhw
Copy link
Contributor

Joshhw commented Jul 27, 2022

Ok, I think I've fixed most things. I noticed the listing jobs kept failing as it was hoping to find react 17 rather than react 18. I also added another peer dep as well. Please someone test this out.

@Joshhw Joshhw requested a review from thiteixeira July 27, 2022 00:40
Copy link
Contributor

@thiteixeira thiteixeira left a comment

Choose a reason for hiding this comment

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

Great work! Just a comment on the react types version, but otherwise it looks great

Screen Shot 2022-07-27 at 9 06 47 AM

Co-authored-by: Thiago Teixeira <thiteixeira@gmail.com>
@Joshhw
Copy link
Contributor

Joshhw commented Aug 2, 2022

Do I need anyone else to approve this?

@thiteixeira
Copy link
Contributor

@mathcolo can't technically approve since he created the PR, but maybe a written approval?

Copy link
Contributor

@idreyn idreyn left a comment

Choose a reason for hiding this comment

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

This looks good (actually, so cool) to me. This is more Docker than I've ever seen, so I'm mostly approving on pure enthusiasm.

Should we update the README with new local build instructions (here or in a follow-up PR?)

@thiteixeira
Copy link
Contributor

I can update the README on my PR (#99) if you like so we get this in

Copy link
Contributor Author

@mathcolo mathcolo left a comment

Choose a reason for hiding this comment

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

I think we should wait on merging this until we resolve the React 18 incompatibility

@Joshhw
Copy link
Contributor

Joshhw commented Aug 3, 2022

I think we should wait on merging this until we resolve the React 18 incompatibility

should be good now. I honestly don't know why I had issues initially but it seems to be fine after updating the versions.

@mathcolo mathcolo merged commit 8dfa2cb into master Aug 4, 2022
@mathcolo mathcolo deleted the containerize branch August 4, 2022 00:48
mathcolo added a commit that referenced this pull request Aug 4, 2022
mathcolo added a commit that referenced this pull request Aug 4, 2022
devinmatte added a commit that referenced this pull request Aug 11, 2023
* container WIP

* add preliminary proposal

* address most pr comments

* functional docker-compose setup

* removes depends_on cause v3 doesn't use it

* remove os import as its unused

* remove favicon dependency and update extension to .ico

* mostly functional now

* adds script for building

* fixes db host and removes it to local env for docker image

* removes unneeded code

* Install poetry instead of pipenv

* trying new config

* remove extra space in util.py

* Update server/history/util.py

Co-authored-by: Thiago Teixeira <thiteixeira@gmail.com>

* updates poetry requirements and some dockerfile stuff along with the lock file

* new version who dis

* fix localhost to 0.0.0.0 for the gunicorn app

* testing this change

* Update package.json

Co-authored-by: Thiago Teixeira <thiteixeira@gmail.com>

* testing new package lock

* update react versions

* fix patch version

* Revert "Revert "Containerize NTT (#90)""

This reverts commit 7ff758f.

* Updating static data and display updated date

* adding api call

* modify line stats based on available data

* revert some changes

* bring back static data

* some change

* match branch with master

* forgot package-lock

* remove console statements

* Update static file

* Using static data again

* Resolve json in typescript

* Train cars not Trains

---------

Co-authored-by: Preston Mueller <skierjunk@gmail.com>
Co-authored-by: Joshua Decosta <josh@Joshuas-M1.lan>
Co-authored-by: Joshua Decosta <joshdecosta@gmail.com>
Co-authored-by: Devin Matte <devinmatte@gmail.com>
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.

6 participants