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 the frontend app & the backend services #25

Merged
merged 8 commits into from
Mar 6, 2022
Merged

Dockerize the frontend app & the backend services #25

merged 8 commits into from
Mar 6, 2022

Conversation

hazemessam
Copy link
Contributor

@hazemessam hazemessam commented Mar 3, 2022

Related Issues or bug

Fixes: #22

image

@Aryamanz29 Aryamanz29 self-requested a review March 3, 2022 07:48
@Aryamanz29 Aryamanz29 added GSSoC22 Counts towards GSSoC22 contribution Level3 Level - Hard (GSSoC22) Ready for Review/Testing Mark PR ready for review/testing labels Mar 3, 2022
@hazemessam hazemessam mentioned this pull request Mar 4, 2022
Copy link
Owner

@Aryamanz29 Aryamanz29 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 @hazemessam 👏

NOTE : Before start working on requested changes, I would highly recommend you to run this project locally and try to upload & search query on sample-log file present in root directory of the project.

Some changes I would suggest before merging this one:

  • I tested your PR and found that docker-compose missing elasticsearch service and it is essential for this project to run properly.

Some references that might help you 👇

  1. https://tamerlan.dev/how-to-integrate-elasticsearch-with-drf/
  2. https://dev.to/aymanemx/building-a-full-text-search-app-using-django-docker-and-elasticsearch-3bai
  3. https://github.com/barseghyanartur/django-elasticsearch-dsl-drf/blob/master/docker-compose.yml
  • Please update CORS_ORIGIN_WHITELIST & ALLOWED HOSTS as well because it's giving me 500 Internal server errors on console.

    CORS_ORIGIN_WHITELIST = [
    "http://127.0.0.1:3000",
    "http://localhost:3000",
    ]

    ALLOWED_HOSTS = []

  • Frontend also not working properly in docker, It's unable to download frontend dependencies through npm install and also make sure you change backend url 👇

    const url = `http://localhost:8000/api/document/`;

  • It would be better if you could attach any kind of GIF/Video in your PR to showcase your work 🙂

@hazemessam
Copy link
Contributor Author

Please, note that we don't need to change the backend address from localhost:8000 to backend:8000 in the frontend, because the frontend will request the backend from the browser and the browser can access the backend container using localhost

@Aryamanz29
Copy link
Owner

Please, note that we don't need to change the backend address from localhost:8000 to backend:8000 in the frontend, because the frontend will request the backend from the browser and the browser can access the backend container using localhost

You're right 👍, It just mapped localhost address. We don't need to change that

Copy link
Owner

@Aryamanz29 Aryamanz29 left a comment

Choose a reason for hiding this comment

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

@hazemessam Great work, Thanks 👍👍

@Aryamanz29 Aryamanz29 removed Ready for Review/Testing Mark PR ready for review/testing requested-changes labels Mar 6, 2022
@Aryamanz29 Aryamanz29 merged commit 38af402 into Aryamanz29:master Mar 6, 2022
@hazemessam hazemessam changed the title Dockerize the frontend & the backend Dockerized the frontend & the backend Mar 7, 2022
@hazemessam hazemessam changed the title Dockerized the frontend & the backend Dockerize the frontend & the backend Mar 7, 2022
@hazemessam hazemessam changed the title Dockerize the frontend & the backend Dockerize the frontend app & the backend services Apr 14, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
GSSoC22 Counts towards GSSoC22 contribution Level3 Level - Hard (GSSoC22)
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Dockerize this project
2 participants