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

160 start aaec sh to use rbenv #204

Closed
wants to merge 2 commits into from

Conversation

Janell-Huyck
Copy link
Contributor

Fixes #160
scripts/start_aaec.sh needs to use rbenv instead of rvm.

Scripts/start_aaec.sh doesn't actually call for the use of either rbenv or rvm. Here's the old version of the file:

#!/bin/bash
#
# Starts the application if it's not already running
#
# Example usage: script/start-aaec.sh /opt/webapps/aaec /aaec production

APP_DIR=$1 # Directory where the app is installed
WEB_URI=$2 # The sub-URI on the web server where the app can be accessed
MODE=$3    # production or development

if ! ( [ -f $APP_DIR/tmp/puma/pid ] && pgrep -F $APP_DIR/tmp/puma/pid > /dev/null )
then
  cd $APP_DIR
  export PATH=$APP_DIR/vendor/bundle/ruby/3.2.2/bin:/usr/local/bin:$PATH
  export RAILS_ENV=$MODE
  export RAILS_RELATIVE_URL_ROOT=$WEB_URI
  bin/bundle exec puma -d
fi

Here's what ChatGPT had to say about the script:
Certainly, let's walk through the Bash script line by line to understand what it's doing:

  1. #!/bin/bash: This is the shebang line, telling the system that the script should be executed with the bash shell.

  2. # Starts the application if it's not already running: A comment line to describe what the script does.

  3. # Example usage: script/start-aaec.sh /opt/webapps/aaec /aaec production: Another comment providing an example of how to run the script.

  4. APP_DIR=$1: Sets the variable APP_DIR to the value of the first argument passed to the script. This is the directory where the app is installed.

  5. WEB_URI=$2: Sets WEB_URI to the second argument, which is the sub-URI where the app can be accessed.

  6. MODE=$3: Sets MODE to the third argument, which specifies whether the app is running in "production" or "development" mode.

  7. if ! ( [ -f $APP_DIR/tmp/puma/pid ] && pgrep -F $APP_DIR/tmp/puma/pid > /dev/null ): This is an if condition that checks if the Puma server (commonly used with Rails) is not running. It does so by checking if a PID file exists and if a process with that PID is running.

  8. then: Starts the block of code to execute if the if condition is true.

  9. cd $APP_DIR: Changes the current directory to the app directory specified by APP_DIR.

  10. export PATH=$APP_DIR/vendor/bundle/ruby/3.2.2/bin:/usr/local/bin:$PATH: Sets the PATH environment variable to include the Ruby 3.2.2 bundled executables directory and other directories.

  11. export RAILS_ENV=$MODE: Sets the Rails environment (RAILS_ENV) to the MODE specified (either "production" or "development").

  12. export RAILS_RELATIVE_URL_ROOT=$WEB_URI: Sets the relative URL root for the Rails app.

  13. bin/bundle exec puma -d: Runs Puma in daemon mode (-d) using the bundled Ruby.

  14. fi: Ends the if block.

This script doesn't specifically use RVM or rbenv. The Ruby version and dependencies are set using the PATH environment variable, which is set to look at the Ruby executable in the vendor/bundle/ruby/3.2.2/bin directory.

...

The line export PATH=$APP_DIR/vendor/bundle/ruby/3.2.2/bin:/usr/local/bin:$PATH is more indicative of a "bundled" Ruby setup, potentially used in deployment environments to isolate dependencies. This is often seen in environments where you bundle Ruby and its gems together with your application code, such as in certain Docker or Capistrano setups.

In summary, the setting of the PATH in the script doesn't align well with how rbenv or RVM usually manage Ruby versions, and it's more indicative of a setup where the Ruby version and gems are bundled directly with the application.

SO!!!

I have not modified the scripts/start-aaec.sh file. I DID notice, however, that the scripts/start-local.sh file was expecting the user to be using rvm. Since it's possible that our developers might be using RVM and it shouldn't really matter for the sake of development, I updated the start-local.sh script to try to start the app with either rbenv or rvm versions, and to attempt to install a missing Ruby version 3.2.2 if asked.

@crowesn
Copy link
Contributor

crowesn commented Sep 13, 2023

See updated description on PR #160

@Janell-Huyck Janell-Huyck added Do Not Merge This PR request should not be merged. and removed Do Not Merge This PR request should not be merged. labels Sep 19, 2023
@Janell-Huyck
Copy link
Contributor Author

Janell-Huyck commented Sep 19, 2023

New issue number 214 created by Sean to address puma updates. The pr associated with this issue (#160), number #204, is not directly related to the original issue, which is no longer relevant. number 214 will still need to be addressed.

@crowesn crowesn self-assigned this Sep 22, 2023
@crowesn
Copy link
Contributor

crowesn commented Sep 22, 2023

Upon looking more closely at #160, I see now that the scripts/start_aaec.sh script isn't directly tied to rbenv, but it's also now no longer relevant since we have configured puma as a service on qa and production. I'd like this to be a clean PR specifically addressing the start_aaec.sh script and I'd like us to take up the work on the start_local script on a different issue/PR.

For this PR, I think we should create a commit where we:

  1. Delete scripts/start_aaec.sh since it is no longer relevant.
  2. Checkout/reset the changes you've made to scripts/start_local.sh and address in another PR.

Once we have a clean PR with above, I think we should open a new issue outlining criteria for updating the scripts/start_local.sh and move the work on start_local to it's PR referencing that issue.

@Janell-Huyck
Copy link
Contributor Author

Closing - PR #222 removes the start aaec.sh script. #224 removes the start local script.

@Janell-Huyck Janell-Huyck deleted the 160-start-aaec-sh-to-use-rbenv branch September 26, 2023 12:28
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Remove start-aaec.sh script
2 participants