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

Containerize #505

Open
wants to merge 7 commits into
base: development
Choose a base branch
from
Open

Containerize #505

wants to merge 7 commits into from

Conversation

kmehant
Copy link

@kmehant kmehant commented Jul 6, 2020

Signed-off-by: K mehant 411843@student.nitandhra.ac.in

Todo list:

  • container config for publishing
  • container config for local development
  • GitHub workflow for build and push container image
  • docker-compose file to set up the complete project instantly (docker-compose up command)
  • Create docker hub account for Codeuino Donut
  • Add GitHub secrets DOCKER_USERNAME and DOCKER_PASSWORD to this repository
  • Write docs for developers to make use of development docker file

kmehant added 2 commits July 6, 2020 22:48
Signed-off-by: K mehant <411843@student.nitandhra.ac.in>
Signed-off-by: K mehant <411843@student.nitandhra.ac.in>
@netlify
Copy link

netlify bot commented Jul 6, 2020

Deploy preview for donut-frontend failed.

Built with commit f67e3cb

https://app.netlify.com/sites/donut-frontend/deploys/5f07415aaa833e000876b193

Signed-off-by: K mehant <411843@student.nitandhra.ac.in>

WORKDIR /server/social-platform-donut-frontend

RUN npm install && \
Copy link
Member

Choose a reason for hiding this comment

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

be verbose with npm version

Copy link
Author

@kmehant kmehant Jul 6, 2020

Choose a reason for hiding this comment

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

@vaibhavdaren
Are you pointing to the tag --loglevel ?

Copy link
Member

Choose a reason for hiding this comment

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

Copy link
Author

Choose a reason for hiding this comment

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

@vaibhavdaren Thanks for the link, I have changed node from LTS to 14 so this will be always using a specific version of npm which is v6 (Major).
However, the link mentions about using a higher version of npm outside the lower node versions. I guess now we have got v14 (three years later) with stringent npm v6 within the container. So. we no need to install again npm as per the link. Does this sound right?

Signed-off-by: K mehant <411843@student.nitandhra.ac.in>
@vaibhavdaren
Copy link
Member

Please attach readme link here

@kmehant
Copy link
Author

kmehant commented Jul 8, 2020

@vaibhavdaren Please let me know any other tests we can write or have to for docker files (PS: I am not sure of any other than using a linter) other than testing locally or within the Actions pipeline by building the image.

@kmehant kmehant requested a review from vaibhavdaren July 8, 2020 14:02
@vaibhavdaren
Copy link
Member

I means that we need to test locally before merging the pr. Thats it.

@kmehant
Copy link
Author

kmehant commented Jul 9, 2020

@vaibhavdaren Please don't merge, I need to fix the pipeline a bit 😅

@vaibhavdaren
Copy link
Member

Please ask @jaskiratsingh2000 to run locally and merge.

@vaibhavdaren vaibhavdaren changed the title Containerize WIP: Containerize Jul 9, 2020
Signed-off-by: K mehant <411843@student.nitandhra.ac.in>
@kmehant kmehant changed the title WIP: Containerize Containerize Jul 9, 2020
@kmehant
Copy link
Author

kmehant commented Jul 9, 2020

Please ask @jaskiratsingh2000 to run locally and merge.

@vaibhavdaren
I made the fix, actually, we had the same problem with the backend so I fixed that. Now the pipeline should work.
Please put your review again. Thank you.

@jaskiratsingh2000 Can you please review this by locally running this feature, I have done it from my side anyways.

@jaskiratsingh2000
Copy link
Member

I would be able to do probably by wednesday. @devesh-verma will respond soon

Signed-off-by: K mehant <411843@student.nitandhra.ac.in>
@jaskiratsingh2000
Copy link
Member

Hi, @kmehant After struggling so much with my docker desktop and testing it threw an error for me. I have 2 major concerns:

  • The information you have provided here seems to be very vague as if the command " $ docker-compose -f docker-compose.dev.yml up --build " needs to be entered then what and how one should go about it is missing from your documentation. You have not mentioned where to write too. So from the non-technical point of view, it becomes difficult to test.
  • On running the above following command it shows me an error of "command not found"

Thanks

@kmehant
Copy link
Author

kmehant commented Aug 15, 2020

@jaskiratsingh2000 FYI -

  • Docker is a DevOps tool so installing, maintaining and using will be little difficult for non-technical or completely frontend devs. I also agree that it is not just installing, using and maintaining it on your machine is also important as it a Daemon process itself with os level virtualization involved. So clearing up volumes, large images running containers, cached containers and many more which use lots of system resources. So I admit it's not a easy process to follow. That does not mean that our documentation should educate people about DevOps, container and Docker tool.

  • Does the community include DevOps people? I would request you to give this testing responsibility to a DevOps guy.

Answering your concerns:
Commands are usually meant to go to a terminal, also please notice $ sign which means it should go to your terminal on Mac or Linux and git bash on Windows. I assume every dev guy would know about terminal and commands which is similar to running npm install as mentioned in DONUT docs.
**Command not found ** means docker-compose command is not present on your machine, you might have not installed docker machine properly or you might typed the wrong command I guess you might have added $

Any more concerns please let me know.
Thank you.

@vaibhavdaren vaibhavdaren self-requested a review August 26, 2020 05:54
Copy link
Member

@vaibhavdaren vaibhavdaren left a comment

Choose a reason for hiding this comment

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

@kmehant @jaskiratsingh2000 Please re-iterate on docker images.

[1] Proper documentation
[2] Test cases are running
[3] Recreate-DB or loading sample database works
cc @Rupeshiya @AuraOfDivinity

After docker-compose what steps are missing? ==> before a new contributor can start the development?

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.

3 participants