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

Add mounts for docker-compose-dev-services #33

Merged
merged 2 commits into from
Feb 29, 2024

Conversation

cmltaWt0
Copy link
Contributor

@cmltaWt0 cmltaWt0 commented Jan 24, 2024

This PR is adding the ability to do a local development for credentials.

I am trying to use the plugin for local development but the process of adding mounts doesn't work for me.
Am I missing something or mounts should be considered in local-docker-compose-dev-services?

--

How to test:

cat ./root/env/dev/docker-compose.yml
...
  credentials:
    environment:
      DJANGO_SETTINGS_MODULE: credentials.settings.tutor.development
    command: ./manage.py runserver 0.0.0.0:8150
    ports:
      - "127.0.0.1:8150:8150"
    stdin_open: true
    tty: true
    volumes:
      # editable requirements
      - ../plugins/credentials/build/credentials/requirements:/openedx/requirements
    networks:
      default:
        aliases:
          - "credentials.local.edly.io"
...
tutor mounts add ./src/credentials
cat ./root/env/dev/docker-compose.yml
...
  credentials:
    environment:
      DJANGO_SETTINGS_MODULE: credentials.settings.tutor.development
    command: ./manage.py runserver 0.0.0.0:8150
    ports:
      - "127.0.0.1:8150:8150"
    stdin_open: true
    tty: true
    volumes:
      # editable requirements
      - ../plugins/credentials/build/credentials/requirements:/openedx/requirements
      - /Users/cmltawt0/code/boxes/nightly/src/credentials:/openedx/credentials
    networks:
      default:
        aliases:
          - "credentials.local.edly.io"
...

The same effect we can achieve by manually removing our mount from ./root/env/dev/docker-compose.yml and then invoking config save command:

tutor config save
cat ./root/env/dev/docker-compose.yml
...
  credentials:
    environment:
      DJANGO_SETTINGS_MODULE: credentials.settings.tutor.development
    command: ./manage.py runserver 0.0.0.0:8150
    ports:
      - "127.0.0.1:8150:8150"
    stdin_open: true
    tty: true
    volumes:
      # editable requirements
      - ../plugins/credentials/build/credentials/requirements:/openedx/requirements
      - /Users/cmltawt0/code/boxes/nightly/src/credentials:/openedx/credentials
    networks:
      default:
        aliases:
          - "credentials.local.edly.io"
...

I'll test against master branch and post command to make it works with all console output.
Then change the DRAFT status to ready.

@kdmccormick kdmccormick self-requested a review January 29, 2024 15:34
@cmltaWt0 cmltaWt0 changed the title Add mounts for docker-compose-dev-services [DRAFT] Add mounts for docker-compose-dev-services Jan 29, 2024
Copy link
Collaborator

@regisb regisb left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In principle, this is a great new feature! Please add some docs about it to the README. Also, please add a changelog entry https://docs.tutor.overhang.io/tutor.html#contributing

@regisb
Copy link
Collaborator

regisb commented Feb 20, 2024

@Faraz32123 what's the status of this PR?
@cmltaWt0 can you please add a changelog entry? https://docs.tutor.overhang.io/tutor.html#contributing

@regisb regisb changed the title [DRAFT] Add mounts for docker-compose-dev-services Add mounts for docker-compose-dev-services Feb 20, 2024
@Faraz32123
Copy link
Collaborator

Faraz32123 commented Feb 20, 2024

@Faraz32123 what's the status of this PR? @cmltaWt0 can you please add a changelog entry? https://docs.tutor.overhang.io/tutor.html#contributing

@cmltaWt0 was going to test it and then change the name from DRAFT.
It is not ready for review according to this comment

@cmltaWt0
Copy link
Contributor Author

cmltaWt0 commented Feb 20, 2024

@regisb @Faraz32123
Sorry for such delay - lack of time.
I'm going to add a changelog entry and switch the target branch to master.
PR is still in draft.

@regisb regisb marked this pull request as draft February 20, 2024 10:03
@cmltaWt0 cmltaWt0 changed the base branch from nightly to master February 20, 2024 14:05
@cmltaWt0 cmltaWt0 marked this pull request as ready for review February 20, 2024 14:05
@cmltaWt0
Copy link
Contributor Author

@regisb @Faraz32123 I've added the changelog, switched the target branch to master.
The PR can be tested now.

Copy link
Collaborator

@Faraz32123 Faraz32123 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM!

@Faraz32123 Faraz32123 merged commit 63688aa into overhangio:master Feb 29, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Development

Successfully merging this pull request may close these issues.

3 participants