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

Move postAttach and postCreate actions to Dockerfile #29

Closed
Jongmassey opened this issue May 22, 2024 · 3 comments
Closed

Move postAttach and postCreate actions to Dockerfile #29

Jongmassey opened this issue May 22, 2024 · 3 comments
Assignees

Comments

@Jongmassey
Copy link
Contributor

Jongmassey commented May 22, 2024

The research template devcontainer.json contains a postCreateCommand setting and postAttachCommand setting. Currently the former calls .devcontainer/postCreate.sh (which is included in the research template repo), and the latter starts rstudio server.

If we wish to make changes to what happens on either of these events, then this currently requires making changes to the research template which:

  • is harder to deploy to all users
  • is harder to test that concurrent changes to the Dockerfile and devcontainer.json work correctly together

I propose that:

  • postCreate.sh, requirements.in and a new postAttach.sh are added to the image as part of the Dockerfile
  • the devcontainer.json postCreateCommand and postAttachCommand settings call these scripts.
  • postcreate.sh and requirements.in are removed from the research template repository

Acceptance criteria

  • The postAttach and postCreate scripts are moved as described above
  • Documentation is updated for adding a Codespace to an existing project
  • There's an upgrade path for existing Codespaces users.

Note: we should keep an eye on how long this work takes and maybe reconsider if it potentially takes us beyond 28th June.

@StevenMaude StevenMaude self-assigned this Jun 18, 2024
StevenMaude added a commit that referenced this issue Jun 19, 2024
See #29.

These were in the `research-template` but we intend to store them here
to simplify maintenance.

These were taken from
opensafely/research-template@5bd648f.

Note that the `postAttach.sh` only existed as a line in the `devcontainer.json`
and has now been moved to a script.
@StevenMaude
Copy link
Contributor

StevenMaude commented Jun 19, 2024

Repository changes

Documentation changes

I don't think that the documentation for adding Codespaces to a project needs updating The steps to update are the same, and there are still multiple files to copy across to a project.

If we need to describe the upgrade path, the simplest way to do so would be:

  1. Delete the existing files out of .devcontainer in your project.
  2. Repeat the steps for "Adding Codespaces to your project".

PR to explain the upgrade path (which isn't documented):

StevenMaude added a commit to opensafely/documentation that referenced this issue Jun 19, 2024
This adds a brief note to explain a task that isn't already documented.

Relates to opensafely-core/research-template-docker#29 as the changes
there might warrant users upgrading to the latest `.devcontainer`
version in the `research-template` repository.
@StevenMaude
Copy link
Contributor

I think merging the PRs above should be sufficient to close this issue.

StevenMaude added a commit that referenced this issue Jun 19, 2024
See #29.

These were in the `research-template` but we intend to store them here
to simplify maintenance.

These were taken from
opensafely/research-template@5bd648f.

Note that the `postAttach.sh` only existed as a line in the `devcontainer.json`
and has now been moved to a script.
@StevenMaude
Copy link
Contributor

Closing as the related PRs are now merged.

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

No branches or pull requests

2 participants