refactor: deprecate devstack from registrar IDA #39
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
huniafatima-arbi
changed the title
huniafatima-arbi
force-pushed
the
huniafatima/migrate-dockerfile-setup
branch
from
d0ed02b to
c9e1c25
Compare
huniafatima-arbi
force-pushed
the
huniafatima/migrate-dockerfile-setup
branch
from
c9e1c25 to
a416375
Compare
huniafatima-arbi
changed the title
UsamaSadiq
requested changes
Sep 10, 2024
| workflow_dispatch: | ||
|
|
||
| schedule: | ||
| - cron: "0 4 * * *" |
There was a problem hiding this comment.
Let's schedule it to run only on the weekdays and mention the default time zone which will be used i.e. UTC
Suggested change
| - cron: "0 4 * * *" | |
| - cron: "0 4 * * 1-5" # UTC Time |
| result-encoding: string | ||
|
|
||
| - name: Build and push Dev Docker image | ||
| uses: docker/build-push-action@v1 |
There was a problem hiding this comment.
Let's try using the latest available v6 version.
Suggested change
| uses: docker/build-push-action@v1 | |
| uses: docker/build-push-action@v6 |
| username: ${{ secrets.DOCKERHUB_USERNAME }} | ||
| password: ${{ secrets.DOCKERHUB_PASSWORD }} | ||
| target: dev | ||
| repository: edxops/registrar-devstack-dev |
There was a problem hiding this comment.
As the dev environment is solely used within the devstack setup, there is no need to add a redundant identifier. Simply specifying registrar-dev will suffice. Since we will be updating the image, there's no need to distinguish it from previous versions.
Suggested change
| repository: edxops/registrar-devstack-dev | |
| repository: edxops/registrar-dev |
| @@ -535,7 +535,7 @@ services: | |||
| CELERY_BROKER_PASSWORD: password | |||
| DJANGO_WATCHMAN_TIMEOUT: 30 | |||
| ANALYTICS_DASHBOARD_CFG: /edx/etc/registrar.yml | |||
| image: edxops/registrar-dev:${OPENEDX_RELEASE:-latest} | |||
| image: edxops/registrar-devstack-dev:${OPENEDX_RELEASE:-latest} | |||
There was a problem hiding this comment.
This change can be skipped if we decide to use registrar-dev.
Suggested change
| image: edxops/registrar-devstack-dev:${OPENEDX_RELEASE:-latest} | |
| image: edxops/registrar-dev:${OPENEDX_RELEASE:-latest} |
dockerfiles/registrar.Dockerfile
Outdated
|
|
||
| # System requirements. | ||
| RUN apt-get update | ||
| RUN DEBIAN_FRONTEND=noninteractive apt-get install -qy \ |
There was a problem hiding this comment.
we've already defined the flag above so it is not needed here.
Suggested change
| RUN DEBIAN_FRONTEND=noninteractive apt-get install -qy \ | |
| RUN apt-get install -qy \ |
dockerfiles/registrar.Dockerfile
Outdated
|
|
||
| # Gunicorn 19 does not log to stdout or stderr by default. Once we are past gunicorn 19, the logging to STDOUT need not be specified. | ||
| CMD ["gunicorn", "--workers=2", "--name", "registrar", "-c", "/edx/app/registrar/registrar/docker_gunicorn_configuration.py", "--log-file", "-", "--max-requests=1000", "registrar.wsgi:application"] | ||
|
|
There was a problem hiding this comment.
nit: remove extra lines from end of file.
dockerfiles/registrar.Dockerfile
Outdated
| ENV DJANGO_SETTINGS_MODULE registrar.settings.production | ||
|
|
||
| # Gunicorn 19 does not log to stdout or stderr by default. Once we are past gunicorn 19, the logging to STDOUT need not be specified. | ||
| CMD ["gunicorn", "--workers=2", "--name", "registrar", "-c", "/edx/app/registrar/registrar/docker_gunicorn_configuration.py", "--log-file", "-", "--max-requests=1000", "registrar.wsgi:application"] |
There was a problem hiding this comment.
We're currently using gunicorn==23.0.0 in production.txt so the comment can be removed, and the command can be simplified as following:
Suggested change
| CMD ["gunicorn", "--workers=2", "--name", "registrar", "-c", "/edx/app/registrar/registrar/docker_gunicorn_configuration.py", "--log-file", "-", "--max-requests=1000", "registrar.wsgi:application"] | |
| CMD ["gunicorn", "--workers=2", "--name", "registrar", "-c", "/edx/app/registrar/registrar/docker_gunicorn_configuration.py", "--max-requests=1000", "registrar.wsgi:application"] |
|
Closing this as this PR will now be created in public-dockerfiles repo |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Description
This PR adds registrar devstack config and image build to devstack repo.