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

Improve performance and functionality of fin bash #1247

Merged
merged 3 commits into from
Dec 17, 2019

Conversation

gmta
Copy link
Contributor

@gmta gmta commented Dec 17, 2019

When running fin bash, a special value was passed to _exec() which sets the
command to run to an interactive shell. This caused the shell and login scripts
to be started twice, which causes an unnecessary delay before the shell is
available for the user.

This commit changes _exec() in the following ways:

  • Instead of executing cd {path} to switch the working directory, use the
    --workdir option of docker exec instead.
  • Consider __SHELL_INTERACTIVE__ a special case where we only run an
    interactive shell without passing a command to run. This cuts time in half
    for a fin bash invocation.
  • Working directory is now also changed for the web container (when running
    fin bash web).

Jelle Raaijmakers added 3 commits December 17, 2019 10:17
When running `fin bash`, a special value was passed to `_exec()` which sets the
command to run to an interactive shell. This caused the shell and login scripts
to be started twice, which causes an unnecessary delay before the shell is
available for the user.

This commit changes `_exec()` in the following ways:

* Instead of executing `cd {path}` to switch the working directory, use the
  `--workdir` option of `docker exec` instead.
* Consider `__SHELL_INTERACTIVE__` a special case where we only run an
  interactive shell without passing a command to run. This cuts time in half
  for a `fin bash` invocation.
* Working directory is now also changed for the `web` container (when running
  `fin bash web`).
@achekulaev achekulaev added this to To do in 1.14.0 via automation Dec 17, 2019
@achekulaev achekulaev self-requested a review December 17, 2019 13:31
@achekulaev
Copy link
Member

@gmta this is an awesome work!

@achekulaev achekulaev merged commit d4b8df5 into docksal:develop Dec 17, 2019
1.14.0 automation moved this from To do to Done Dec 17, 2019
@lmakarov lmakarov mentioned this pull request Dec 18, 2019
JDDoesDev added a commit to JDDoesDev/docksal that referenced this pull request Jun 27, 2021
* 'master' of github.com:docksal/docksal: (171 commits)
  Updated CONTRIBUTING.md
  Updated LICENSE year
  Updated README
  Updated BLT boilerplate URL
  Updated CHANGELOG
  Bump fin v1.93.0
  Improved messaging in ssh_key()
  Added user and shell labels on cli in sercvice.yml
  Added back xdebug settings workaround for backward compatibility
  Updated fin run-cli docs
  Use docker instead of docker-compose for get_project_container_id() (docksal#1246)
  Improve performance and functionality of `fin bash` (docksal#1247)
  Fix SSH_AUTH_SOCK being unset for custom commands
  Revamp of Xdebug docs (docksal#1242)
  Fix tests
  Cleanup items pending to be removed as per TODO notes
  Add IDE_PASSWORD var to IDE stack file (docksal#1241)
  Updated release data in CHANGELOG
  Mentioned PHP 7.3 is now the default in CHANGELOG
  Bump fin 1.92.2 + CHANGELOG update
  ...
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
No open projects
1.14.0
  
Done
Development

Successfully merging this pull request may close these issues.

None yet

2 participants