RocketChat / RC4Conferences

A set of scalable components for communities to build, manage, and run virtual conferences of any size.
https://conf.rceng.shop/conferences/c/1
24 stars 38 forks source link

Chore: Redundant data in .env file #66

Open Shivam164 opened 1 year ago

Shivam164 commented 1 year ago

On running the command sh startdevenv.sh localhost some data gets added in the env file every time we run it, because of which some redundant data gets accumulated in .env file.

VipinDevelops commented 1 year ago

Hey i would like to work on this issue can you please assign it to me

Dnouv commented 1 year ago

Sure, @VipinDevelops however first, please explain the approach you will take to tackle the issue.

Thank you!

VipinDevelops commented 1 year ago

Yeah sure @Dnouv It looks like the script is modifying the contents of the app/.env file each time it's run, which is causing unnecessary data to accumulate.

One way to prevent the app/.env file from accumulating unnecessary data would be to add a step in the script that clears the file before adding new data to it.

VipinDevelops commented 1 year ago

we can add the following command before the printf statements that are appending data to the file: > app/.env This command will truncate the file to zero length,

VipinDevelops commented 1 year ago

Another approach is to only add the variables that aren't already set in the file before writing the new data to it. we can use the command grep and the -q flag to check whether the variable is already set or not.

VipinDevelops commented 1 year ago

image

VipinDevelops commented 1 year ago

Hey!!
@Dnouv @Shivam164 I have suggested two approaches that can be used to prevent the app/.env file from accumulating unnecessary data each time the script is run. The first approach is to clear the contents of the app/.env file before appending new data to it. The second approach is to only add the variables that aren't already set in the file before writing the new data to it.

I hope this additional information is helpful. Let me know if you have any other questions or if you would like me to clarify any further so I can start working on resolving this issue from all the scripts Thank you.

Dnouv commented 1 year ago

Hey @VipinDevelops

Thanks for sharing your approach. Well, I am afraid both of the approaches are incorrect and need more refining.

  1. First approach:

    we can add the following command before the printf statements that are appending data to the file: app/.env This command will truncate the file to zero length.

Think about this scenario: In RC4Conferences, there is an embedded chat component; in its setup, we require three additional environment variables, one of them is the NEXT_PUBLIC_GOOGLE_CLIENT_ID, so if on every restart of the server using sh startdevenv.sh localhost, the user will have to, again and again, add these extra environment vars. Similarly, there are extra environment vars for the Greenroom components. So don't you think it will be tedious to add this on every run of the script? (Source: doc)

  1. Second approach:

Another approach is to only add the variables that aren't already set in the file before writing the new data to it. we can use the command grep and the -q flag to check whether the variable is already set or not.

If you look carefully in the startdevenv.sh script in this line. You can notice there is a variable STRAPI_PORT which has a free port value, which Strapi cms can occupy.

Now think about this scenario:

You start the development environment of RC4Conferences, and on the first run, your local machine, Port 1337, is vacant. So the environment var NEXT_PUBLIC_STRAPI_URL is set to a value localhost:1337. Now you stop the RC4Conferences and go on to try some new project which starts its service on port 1337, and you left it running.

And you are again back to re-start the RC4Conferences; now your script will notice that "Oh, there is already a value for NEXT_PUBLIC_STRAPI_URL, let's skip it", since the port 1337 is occupied, the Strapi CMS will start its service on 1338, however in the .env you still have the old data, which is localhost:1337. So the NextJS still tries to connect to 1337, which will result in an error.

This is what you faced a few days ago, as we discussed on the Open Server.

Please re-think this issue, and I would suggest going through the scripts carefully so that you can understand how each of the environment variables are set.

Thank you!

Dnouv commented 1 year ago

One approach I can think of is partially clearing out the environment vars, which can have dynamic values, based on the changes in port availability or user changes. So when a user re-runs the script, the script logic will only delete the NEXT_PUBLIC_STRAPI_URL and set a new value, it won't touch the values of user-provided env vars such as NEXT_PUBLIC_GOOGLE_CLIENT_ID.

However, you will require a much better understanding of how the scripts in RC4Conferences work. Thank you!

VipinDevelops commented 1 year ago

Thanks for explaining to me how the flow is working I landed a PR for fixing this issue can you please check that out and tell me what I could do to improve that https://github.com/RocketChat/RC4Conferences/pull/79

Dnouv commented 1 year ago

Please read the above comment carefully. The opened PR implements the second approach, and the problems associated with this approach are mentioned in the comment.

Thank you!

Palanikannan1437 commented 1 year ago

Hey @VipinDevelops

Thanks for sharing your approach. Well, I am afraid both of the approaches are incorrect and need more refining.

  1. First approach:

we can add the following command before the printf statements that are appending data to the file: app/.env This command will truncate the file to zero length.

Think about this scenario: In RC4Conferences, there is an embedded chat component; in its setup, we require three additional environment variables, one of them is the NEXT_PUBLIC_GOOGLE_CLIENT_ID, so if on every restart of the server using sh startdevenv.sh localhost, the user will have to, again and again, add these extra environment vars. Similarly, there are extra environment vars for the Greenroom components. So don't you think it will be tedious to add this on every run of the script? (Source: doc)

  1. Second approach:

Another approach is to only add the variables that aren't already set in the file before writing the new data to it. we can use the command grep and the -q flag to check whether the variable is already set or not.

If you look carefully in the startdevenv.sh script in this line. You can notice there is a variable STRAPI_PORT which has a free port value, which Strapi cms can occupy.

Now think about this scenario:

You start the development environment of RC4Conferences, and on the first run, your local machine, Port 1337, is vacant. So the environment var NEXT_PUBLIC_STRAPI_URL is set to a value localhost:1337. Now you stop the RC4Conferences and go on to try some new project which starts its service on port 1337, and you left it running.

And you are again back to re-start the RC4Conferences; now your script will notice that "Oh, there is already a value for NEXT_PUBLIC_STRAPI_URL, let's skip it", since the port 1337 is occupied, the Strapi CMS will start its service on 1338, however in the .env you still have the old data, which is localhost:1337. So the NextJS still tries to connect to 1337, which will result in an error.

This is what you faced a few days ago, as we discussed on the Open Server.

Please re-think this issue, and I would suggest going through the scripts carefully so that you can understand how each of the environment variables are set.

Thank you!

Loved the scenarios and the detailed explanation @Dnouv ✨

VipinDevelops commented 1 year ago

Hey @Dnouv i have some more ideas for this issues like A simple solution to this problem would be to clear the contents of the ".env" file before running the script each time. This can be done by adding the command "> app/.env" before the printf statements that are appending data to the ".env" file. This command will truncate the file to zero length, effectively clearing it before the new data is added. This way, each time the script is run, only the necessary environment variables will be present in the ".env" file, and any redundant data will be removed. Another approach would be to separate the environment variables for different components into separate files and include them in the script accordingly. This way, you can avoid adding unnecessary environment variables each time and also keep the environment variable files for different components clean and easy to manage.

VipinDevelops commented 1 year ago

Hey @Dnouv can you please review these ideas I would appreciate if can give me some suggestions to solve this issue

Dnouv commented 1 year ago

A simple solution to this problem would be to clear the contents of the ".env" file before running the script each time. This can be done by adding the command "> app/.env" before the printf statements that are appending data to the ".env" file. This command will truncate the file to zero length, effectively clearing it before the new data is added.

In RC4Conferences, there is an embedded chat component; in its setup, we require three additional environment variables, one of them is the NEXT_PUBLIC_GOOGLE_CLIENT_ID, so if on every restart of the server using sh startdevenv.sh localhost, the user will have to, again and again, add these extra environment vars. Similarly, there are extra environment vars for the Greenroom components. So don't you think it will be tedious to add this on every run of the script? (Source: doc)

Another approach would be to separate the environment variables for different components into separate files and include them in the script accordingly. This way, you can avoid adding unnecessary environment variables each time and also keep the environment variable files for different components clean and easy to manage

This sounds good. Could you please elaborate on this a little further? Any ideas on how to implement this?

Thank you!

Palanikannan1437 commented 1 year ago

Hey @Dnouv can you please review these ideas I would appreciate if can give me some suggestions to solve this issue

Please feel free to try out the approach suggested in a comment I left out in your PR...thank you!

VipinDevelops commented 1 year ago

Another approach would be to separate the environment variables for different components into separate files and include them in the script accordingly. This way, you can avoid adding unnecessary environment variables each time and also keep the environment variable files for different components clean and easy to manage

This sounds good. Could you please elaborate on this a little further? Any ideas on how to implement this?

Thank you!

To separate the environment variables for different components into separate files and include them in the script accordingly, we can follow these steps:

Dnouv commented 1 year ago

Thank you for explaining your approach. From what I understood from your message, we will divide the env variables as follows: For components:

For different stages:

Does this not sound a little confusing to have so many variables? What if we have multiple components that require different variables? If I need to understand something in your approach, please let me know.

Thank you!

VipinDevelops commented 1 year ago

Yes, i agree with you @Dnouv having many environment variables can become confusing when the number of components and stages increase. In such cases, it's better to store the variables in a separate file, for example, .env and .env.production, .env.development and .env.component1, .env.component2. This way, we can easily manage different sets of variables and access them as needed. Additionally, we can also use tools like dotenv to automatically load these environment variables into your application.
Do let me know if you have any suggestions for me so I can come up with best approach Thank you!

VipinDevelops commented 1 year ago

Yes, i agree with you @Dnouv having many environment variables can become confusing when the number of components and stages increase. In such cases, it's better to store the variables in a separate file, for example, .env and .env.production, .env.development and .env.component1, .env.component2. This way, we can easily manage different sets of variables and access them as needed. Additionally, we can also use tools like dotenv to automatically load these environment variables into your application. Do let me know if you have any suggestions for me so I can come up with best approach Thank you!

Hey @Dnouv can you please review this approach