Hey, @Soda I would like to work on this task

Hi and thank you for your interest! Please check thoroughly https://www.mediawiki.org/wiki/New_Developers (and all of its communication section!). The page covers how to get started, assigning tasks, task status, how to find a codebase, how to create patches, where to ask general development questions and where to get help with setup problems, and how to ask good questions. Thanks a lot! :)

@Soda If I'm not mistaken, what we want is to first build the React frontend code, and then serve it from the root route '/' while using a prefix like '/api' for backend-related tasks on the same port

In T332517#8709117, @AFZL210 wrote:

@Soda If I'm not mistaken, what we want is to first build the React frontend code, and then serve it from the root route '/' while using a prefix like '/api' for backend-related tasks on the same port

We use three api prefixes in production, /, /api and /socket.io. Also I believe we still need hot reloading so pre-building React code is kinda a no-go

In T332517#8709531, @Soda wrote:

In T332517#8709117, @AFZL210 wrote:

@Soda If I'm not mistaken, what we want is to first build the React frontend code, and then serve it from the root route '/' while using a prefix like '/api' for backend-related tasks on the same port

We use three api prefixes in production, /, /api and /socket.io. Also I believe we still need hot reloading so pre-building React code is kinda a no-go

sure i'll keep that in mind

@Soda In docker-compose.dev..yml no different services are defined like client and api . in videocuttool itself both the ports are defined for frontend and backend.

Hey @Soda @Gopavasanth My frontend and backend both are running and also nginx is running successfully but I am getting 502 bad gateway error on localhost:8000 (proxy server) . Can you please guide me what should I do

In T332517#8714804, @Aditiganvir28 wrote:

Hey @Soda @Gopavasanth My frontend and backend both are running and also nginx is running successfully but I am getting 502 bad gateway error on localhost:8000 (proxy server) . Can you please guide me what should I do

Based on my understanding that means that the nginx is unable to reach your backend and frontend

@Reputation22 I think @Aditiganvir28 was working on this ?

In T332517#8719611, @Soda wrote:

@Reputation22 I think @Aditiganvir28 was working on this ?

There were no assignees to the task.. so i thought maybe the issue is still open :(

Change 901641 had a related patch set uploaded (by Sohom Datta; author: Aditiganvir28):

[labs/tools/VideoCutTool@master] Made the development server to run on the same port

https://gerrit.wikimedia.org/r/901641

@Soda Is the code verified or should I make changes in it

I have left comments about improvements that need to be made

Actually If I used just node:16 , 'Run apk update' is showing error so I have to use node:16-alpine so that it doesn't show error

In T332517#8725985, @Soda wrote:

I have left comments about improvements that need to be made

I will make the changes

In T332517#8725986, @Aditiganvir28 wrote:

Actually If I used just node:16 , 'Run apk update' is showing error so I have to use node:16-alpine so that it doesn't show error

Why do you need apk update ? We can just use apt to do the same thing

In T332517#8726508, @Soda wrote:

In T332517#8725986, @Aditiganvir28 wrote:

Actually If I used just node:16 , 'Run apk update' is showing error so I have to use node:16-alpine so that it doesn't show error

Why do you need apk update ? We can just use apt to do the same thing

Okay, I will do that

@Soda I have made the required imporvements you told and now the backend and frontend are both running on localhost:8000.
Should I make the patch request again

https://gerrit.wikimedia.org/r/c/labs/tools/VideoCutTool/+/901641/ has no updated version yet, so nothing can be reviewed yet. Please put code changes into Gerrit so folks can review proposed code changes. Please do not manually announce Gerrit patch updates in Phabricator tasks; Gerrit already provides notifications itself. Thanks!

Yeah sure should I push to the same branch

That was already answered by the link in my previous comment

@Soda I am getting this error when I am trying to squash the commits

To https://gerrit.wikimedia.org/r/labs/tools/VideoCutTool
! [remote rejected] HEAD -> refs/for/master%topic=T332517 (no new changes)
error: failed to push some refs to 'https://gerrit.wikimedia.org/r/labs/tools/VideoCutTool'

Can you please guide me regarding this error

@Aditiganvir28: Not in this task (off-topic), and not without full steps to reproduce without any paraphrasing. :) Please see and follow https://www.mediawiki.org/wiki/New_Developers#Communication_tips and bring such questions up in Zulip instead. Thanks.

@Soda I had squash the two commits can you please review it

@Aditiganvir28: Which patch to review? https://gerrit.wikimedia.org/r/c/labs/tools/VideoCutTool/+/901641/ or https://gerrit.wikimedia.org/r/c/labs/tools/VideoCutTool/+/902612/ ? Please do abandon the patch in Gerrit that is not to be reviewed. Please always follow https://www.mediawiki.org/wiki/Gerrit/Commit_message_guidelines for https://gerrit.wikimedia.org/r/c/labs/tools/VideoCutTool/+/902612/1 (correct verb tempus and use imperative mood; always link to the related Phabricator task; remove unneeded sentences like "I had made the changes as per the required improvements needed" which is always the case anyway). Thanks.

This one
https://gerrit.wikimedia.org/r/c/labs/tools/VideoCutTool/+/902612
Patchset 3

@Aditiganvir28 Please read my entire previous comment, and act on all items I listed. Thanks!

Yeah, I am sorry for that

Change 901641 abandoned by Aditiganvir28:

[labs/tools/VideoCutTool@master] Made the development server to run on the same port

Reason:

-

https://gerrit.wikimedia.org/r/901641

Change 902612 had a related patch set uploaded (by Aklapper; author: Aditiganvir28):

[labs/tools/VideoCutTool@master] Development environment is using only one port

https://gerrit.wikimedia.org/r/902612

@Soda Kindly please review my code

In T332517#8726680, @Aditiganvir28 wrote:

Yeah sure should I push to the same branch

Can you mention which change is final, you have two patches for the same change

In T332517#8731599, @Soda wrote:

In T332517#8726680, @Aditiganvir28 wrote:

Yeah sure should I push to the same branch

Can you mention which change is final, you have two patches for the same change

This one
https://gerrit.wikimedia.org/r/c/labs/tools/VideoCutTool/+/902612
Patchset 5

@Soda Kindly please review my code
https://gerrit.wikimedia.org/r/c/labs/tools/VideoCutTool/+/902612
Patchset 5

@Soda do we need hot reloading for both frontend and backend

@Soda I had uploaded the patch for review. Can you please review it.
https://gerrit.wikimedia.org/r/c/labs/tools/VideoCutTool/+/904589

@Aditiganvir28: Again, please stop creating new patches for the same issue in Gerrit. Please fix existing patches instead by amending: https://www.mediawiki.org/wiki/Gerrit/Tutorial#Amending_a_change_.28your_own_or_someone_else.27s.29 . Please abandon patches in Gerrit if they are not relevant anymore. Thanks.

In T332517#8747765, @Aklapper wrote:

@Aditiganvir28: Again, please stop creating new patches for the same issue in Gerrit. Please fix existing patches instead by amending: https://www.mediawiki.org/wiki/Gerrit/Tutorial#Amending_a_change_.28your_own_or_someone_else.27s.29 . Please abandon patches in Gerrit if they are not relevant anymore. Thanks.

Yeah, I am sorry for that

@Soda I had added the new patch for review. Kindly please review it.
https://gerrit.wikimedia.org/r/c/labs/tools/VideoCutTool/+/907830

Change 907830 had a related patch set uploaded (by Aditiganvir28; author: Aditiganvir28):

[labs/tools/VideoCutTool@master] Development environment is running on localhost:8080 using nginx

https://gerrit.wikimedia.org/r/907830

@Soda If I am removing dockerfile.dev form nginx folder and docker-compose.dev.yml it is giving me error.

What error are you getting

In T332517#8783597, @Soda wrote:

What error are you getting

This is the error I am getting
failed to solve: rpc error: code = Unknown desc = failed to solve with frontend dockerfile.v0: failed to read dockerfile: open /var/lib/docker/tmp/buildkit-mount442912650/Dockerfile: no such file or directory

@Soda I tried removing dockerfile from nginx folder. But instead of that I have to run the nginx command
CMD ["sh", "-c", "nginx -g 'daemon off;' & npm run dev"].
Should I also bring default.conf file in the main directory instead of creating nginx folder.

Change 908874 had a related patch set uploaded (by Aditiganvir28; author: Aditiganvir28):

[labs/tools/VideoCutTool@master] Development environment is running on localhost:8080 using nginx

https://gerrit.wikimedia.org/r/908874

Change 907830 abandoned by Aditiganvir28:

[labs/tools/VideoCutTool@master] Development environment is running on localhost:8080 using nginx

Reason:

https://gerrit.wikimedia.org/r/907830

In T332517#8783736, @Aditiganvir28 wrote:

@Soda I tried removing dockerfile from nginx folder. But instead of that I have to run the nginx command
CMD ["sh", "-c", "nginx -g 'daemon off;' & npm run dev"].
Should I also bring default.conf file in the main directory instead of creating nginx folder.

I don't have a problem with a different container, what I don't understand is why we need a new dockerfile for it

Also please stop uploading improvements as new patches, that is not how Gerrit is supposed to be used, and causes multiple spam commit/entries. Please unabandon your last change and upload your change as a patchset on top of the old change as per the guide linked in the description.

In T332517#8784210, @Soda wrote:

Also please stop uploading improvements as new patches, that is not how Gerrit is supposed to be used, and causes multiple spam commit/entries. Please unabandon your last change and upload your change as a patchset on top of the old change as per the guide linked in the description.

Yeah, I am sorry for the inconvenience caused. I will create a patchset.

Change 907830 restored by Aditiganvir28:

[labs/tools/VideoCutTool@master] Development environment is running on localhost:8080 using nginx

https://gerrit.wikimedia.org/r/907830

In T332517#8784209, @Soda wrote:

In T332517#8783736, @Aditiganvir28 wrote:

@Soda I tried removing dockerfile from nginx folder. But instead of that I have to run the nginx command
CMD ["sh", "-c", "nginx -g 'daemon off;' & npm run dev"].
Should I also bring default.conf file in the main directory instead of creating nginx folder.

I don't have a problem with a different container, what I don't understand is why we need a new dockerfile for it

AFAIK If we are not using a separate dockerfile then we have to run the nginx server after running our react app and backend. At first it was showing error running both but the issue was I didn't installed nginx in Dockerfile.dev. Now what I did is using only one container. If we want to use different containers we can do that also. In the new patchset upoladed I have used only one container.

In T332517#8784239, @Aditiganvir28 wrote:

In T332517#8784209, @Soda wrote:

In T332517#8783736, @Aditiganvir28 wrote:

@Soda I tried removing dockerfile from nginx folder. But instead of that I have to run the nginx command
CMD ["sh", "-c", "nginx -g 'daemon off;' & npm run dev"].
Should I also bring default.conf file in the main directory instead of creating nginx folder.

I don't have a problem with a different container, what I don't understand is why we need a new dockerfile for it

AFAIK If we are not using a separate dockerfile then we have to run the nginx server after running our react app and backend. At first it was showing error running both but the issue was I didn't installed nginx in Dockerfile.dev. Now what I did is using only one container. If we want to use different containers we can do that also. In the new patchset upoladed I have used only one container.

I would prefer it to be a different container

In T332517#8784244, @Soda wrote:

In T332517#8784239, @Aditiganvir28 wrote:

In T332517#8784209, @Soda wrote:

In T332517#8783736, @Aditiganvir28 wrote:

@Soda I tried removing dockerfile from nginx folder. But instead of that I have to run the nginx command
CMD ["sh", "-c", "nginx -g 'daemon off;' & npm run dev"].
Should I also bring default.conf file in the main directory instead of creating nginx folder.

I don't have a problem with a different container, what I don't understand is why we need a new dockerfile for it

AFAIK If we are not using a separate dockerfile then we have to run the nginx server after running our react app and backend. At first it was showing error running both but the issue was I didn't installed nginx in Dockerfile.dev. Now what I did is using only one container. If we want to use different containers we can do that also. In the new patchset upoladed I have used only one container.

I would prefer it to be a different container

Can you please review the patch I made two different containers

@Aditiganvir28: There are two open changesets in Gerrit. Please abandon the one that's not relevant.

Change 908874 abandoned by Aditiganvir28:

[labs/tools/VideoCutTool@master] Development environment is running on localhost:8080 using nginx

Reason:

https://gerrit.wikimedia.org/r/908874

@Aditiganvir28: Please strip unneeded quotes. Your comment is unrelated to all the stuff that you quoted. Please keep things readable - thanks!

@Soda Can you please review my patch.
https://gerrit.wikimedia.org/r/c/labs/tools/VideoCutTool/+/907830

@Aditiganvir28 It still seems to have npm processes, can we not do this with only two containers, one with the npm servers and one with the nginx server ? The architecture I was looking for was to have one service with both npm servers and one with only a nginx connection

In T332517#8801563, @Soda wrote:

@Aditiganvir28 It still seems to have npm processes, can we not do this with only two containers, one with the npm servers and one with the nginx server ? The architecture I was looking for was to have one service with both npm servers and one with only a nginx connection

I think for that we need two different dockerfiles one for nginx server and another for npm servers like I did earlier. When I am trying to run both the containers in one dockerfile only npm processes seems to running.

What do you mean ? I'm pretty sure you can run different processes in the same dockerfile as a container. I did a basic test on my PC and was able to come up with a basic config that worked.

version: '3.9'
services:
  videocuttool:
    container_name: videocuttool
    build:
      context: .
      dockerfile: Dockerfile.dev
    volumes:
      - ./src:/app/src:ro
      - ./server:/app/server
      - ./public:/app/public
      - '/app/node_modules'
      - '/app/server/node_modules'
    ports:
      - '3000:3000'
      - '4000:4000'
    environment:
      - CHOKIDAR_USEPOLLING=true
      - NODE_ENV=development
      - DB_CONNECTION_URL=mongodb://videocuttool-mongo:27017/video-cut-tool
      - BACKEND_URL=http://videocuttool:4000/api/
    env_file:
      - .env.dev
    depends_on:
      - mongo
  mongo:
    container_name: videocuttool-mongo
    image: 'mongo:4'
    volumes:
      - ./mongo-data:/data/db
    ports:
      - '27017:27017'
  nginx:
    restart: always
    image: nginx:latest
    depends_on:
      - videocuttool
    ports:
      - 80:80
    volumes:
      - ./nginx/default.conf:/etc/nginx/conf.d/default.conf

In T332517#8803507, @Soda wrote:

What do you mean ? I'm pretty sure you can run different processes in the same dockerfile as a container. I did a basic test on my PC and was able to come up with a basic config that worked.

version: '3.9'
services:
  videocuttool:
    container_name: videocuttool
    build:
      context: .
      dockerfile: Dockerfile.dev
    volumes:
      - ./src:/app/src:ro
      - ./server:/app/server
      - ./public:/app/public
      - '/app/node_modules'
      - '/app/server/node_modules'
    ports:
      - '3000:3000'
      - '4000:4000'
    environment:
      - CHOKIDAR_USEPOLLING=true
      - NODE_ENV=development
      - DB_CONNECTION_URL=mongodb://videocuttool-mongo:27017/video-cut-tool
      - BACKEND_URL=http://videocuttool:4000/api/
    env_file:
      - .env.dev
    depends_on:
      - mongo
  mongo:
    container_name: videocuttool-mongo
    image: 'mongo:4'
    volumes:
      - ./mongo-data:/data/db
    ports:
      - '27017:27017'
  nginx:
    restart: always
    image: nginx:latest
    depends_on:
      - videocuttool
    ports:
      - 80:80
    volumes:
      - ./nginx/default.conf:/etc/nginx/conf.d/default.conf

Oh okay I made a mistake of using build instead of image in services that was resulting in npm processes in both the containers I
guess. I made the changes and uploaded the new patch.

@Soda Can you please review my patch
https://gerrit.wikimedia.org/r/c/labs/tools/VideoCutTool/+/907830

Change 907830 merged by jenkins-bot:

[labs/tools/VideoCutTool@master] Add nginx support to dev env

https://gerrit.wikimedia.org/r/907830