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

Dockerize Parser service #3302

Merged
merged 1 commit into from
Mar 25, 2022
Merged

Conversation

TueeNguyen
Copy link
Contributor

Issue This PR Addresses

Fixes #2111

Type of Change

  • Bugfix: Change which fixes an issue
  • New Feature: Change which adds functionality
  • Documentation Update: Change which improves documentation
  • UI: Change which improves UI

Description

  • Revert all commented parser's docker code
  • Add code to fit in with recent docker changes
  • Rework parser's docker file to better practice

Steps to test the PR

  • pnpm services:start
  • go to localhost:8000 AND THAT'S ALL YOU NEED, NO pnpm start

Checklist

  • Quality: This PR builds and passes our npm test and works locally
  • Tests: This PR includes thorough tests or an explanation of why it does not
  • Screenshots: This PR includes screenshots or GIFs of the changes made or an explanation of why it does not (if applicable)
  • Documentation: This PR includes updated/added documentation to user exposed functionality or configuration variables are added/changed or an explanation of why it does not(if applicable)

@gitpod-io
Copy link

gitpod-io bot commented Mar 24, 2022

@TueeNguyen TueeNguyen requested review from Kevan-Y and TDDR March 24, 2022 01:12
@TueeNguyen TueeNguyen changed the title Parser docker Dockerize Parser serivce Mar 24, 2022
@TueeNguyen TueeNguyen changed the title Dockerize Parser serivce Dockerize Parser service Mar 24, 2022
@TueeNguyen TueeNguyen self-assigned this Mar 24, 2022
@TueeNguyen TueeNguyen added this to the 2.9 Release milestone Mar 24, 2022
@TueeNguyen
Copy link
Contributor Author

Wait for #3040 to merge first

# parser:
# restart: unless-stopped
parser:
image: docker.cdot.systems/posts:${DOCKER_DEPLOY_TAG:-latest}
Copy link
Contributor

Choose a reason for hiding this comment

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

should be parser instead of posts

src/api/parser/Dockerfile Outdated Show resolved Hide resolved
src/api/parser/Dockerfile Outdated Show resolved Hide resolved
@Kevan-Y
Copy link
Contributor

Kevan-Y commented Mar 24, 2022

Image size compare to before

❯ docker images | grep refactor/parser
refactor/parser                                 old                de119410c427   14 seconds ago       185MB
refactor/parser                                 new                5efd1346e57b   About a minute ago   170MB

@TueeNguyen
Copy link
Contributor Author

Looking for another round of review, probably after #3040 this gets merged to see how if the containers will be built correctly.

@TueeNguyen
Copy link
Contributor Author

Image size compare to before

❯ docker images | grep refactor/parser
refactor/parser                                 old                de119410c427   14 seconds ago       185MB
refactor/parser                                 new                5efd1346e57b   About a minute ago   170MB

Small but good improvement

docker/development.yml Outdated Show resolved Hide resolved
docker/gitpod.yml Outdated Show resolved Hide resolved
src/api/parser/Dockerfile Outdated Show resolved Hide resolved
@Kevan-Y Kevan-Y requested a review from humphd March 24, 2022 23:34
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Write tests and enable Parser service
4 participants