Skip to content
This repository has been archived by the owner on Mar 19, 2024. It is now read-only.

edx_notes_api config for native image #993

Open
wants to merge 5 commits into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from 3 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 3 additions & 0 deletions Makefile
Original file line number Diff line number Diff line change
Expand Up @@ -453,6 +453,9 @@ dev.shell.analyticsapi:
dev.shell.insights:
docker-compose exec insights env TERM=$(TERM) bash -c 'eval $$(source /edx/app/insights/insights_env; echo PATH="$$PATH";) && /bin/bash'

dev.shell.edx_notes_api:
Copy link
Contributor

@aht007 aht007 Dec 9, 2022

Choose a reason for hiding this comment

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

If there is no specific make target for note_api then according to my understanding this command will be executed which means there should be no need to add this command explicitly. Can you please try this theory too when testing the Ansible free image

docker-compose exec edx_notes_api env TERM=$(TERM) bash -c 'eval $$(source /edx/app/edx_notes_api/edx_notes_api_env; echo PATH="$$PATH";) && /bin/bash'

dev.shell.%: ## Run a shell on the specified service's container.
docker-compose exec $* /bin/bash

Expand Down
36 changes: 36 additions & 0 deletions configuration_files/edx_notes_api.yml
Original file line number Diff line number Diff line change
@@ -0,0 +1,36 @@
---

ALLOWED_HOSTS:
- localhost
CLIENT_ID: CHANGEME
CLIENT_SECRET: CHANGEME
DATABASES:
default:
ENGINE: django.db.backends.mysql
HOST: db
NAME: edx_notes_api
OPTIONS:
connect_timeout: 10
PASSWORD: secret
PORT: '3306'
USER: notes001
DISABLE_TOKEN_CHECK: false
ELASTICSEARCH_INDEX: edx_notes
ELASTICSEARCH_URL: http://es:9200/
HAYSTACK_CONNECTIONS:
default:
ENGINE: notesserver.highlight.ElasticsearchSearchEngine
INDEX_NAME: edx_notes_api
URL: http://es:9200/
JWT_AUTH:
JWT_AUTH_COOKIE_HEADER_PAYLOAD: edx-jwt-cookie-header-payload
JWT_AUTH_COOKIE_SIGNATURE: edx-jwt-cookie-signature
JWT_ISSUERS:
- AUDIENCE: SET-ME-PLEASE
ISSUER: http://127.0.0.1:8000/oauth2
SECRET_KEY: SET-ME-PLEASE
JWT_PUBLIC_SIGNING_JWK_SET: ''
RESULTS_DEFAULT_SIZE: 25
RESULTS_MAX_SIZE: 250
SECRET_KEY: CHANGEME
USERNAME_REPLACEMENT_WORKER: OVERRIDE THIS WITH A VALID USERNAME
Copy link
Contributor

Choose a reason for hiding this comment

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

nit: no new line

Copy link
Contributor Author

Choose a reason for hiding this comment

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

done

4 changes: 4 additions & 0 deletions docker-compose.yml
Original file line number Diff line number Diff line change
Expand Up @@ -390,6 +390,7 @@ services:
- lms
- mysql57
image: edxops/notes:${OPENEDX_RELEASE:-latest}
Copy link
Contributor

Choose a reason for hiding this comment

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

We have to use new Ansible free image here and test it before we merge this PR

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Can you please help me to find it how you have done it before for other PRs?
I followed analytics PR, so is there anything which I'm missing from analytics PR?

Copy link
Contributor

Choose a reason for hiding this comment

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

We will need to change the image specified here currently with the new Ansible free image once we merge the PR in notes-api repository

Copy link
Contributor Author

Choose a reason for hiding this comment

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

image name changed

working_dir: /edx/app/edx_notes_api/edx_notes_api
networks:
default:
aliases:
Expand All @@ -407,6 +408,9 @@ services:
ENABLE_DJANGO_TOOLBAR: 1
ELASTICSEARCH_URL: "http://edx.devstack.elasticsearch710:9200"
ELASTICSEARCH_DSL: "http://edx.devstack.elasticsearch710:9200"
volumes:
- /edx/var/edx_notes_api/
Copy link
Contributor

Choose a reason for hiding this comment

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

This wasn't present before. We shouldn't add anything not related to our work

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Copy link
Contributor

Choose a reason for hiding this comment

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

I didn't add it as part of my PR. We simply don't need it for notes-api if it wasn't specified already

Copy link
Contributor Author

Choose a reason for hiding this comment

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

thanks, verified it. It was not there before.

- ${PWD}/configuration_files/edx_notes_api.yml:/edx/etc/edx_notes_api.yml

forum:
command: bash -c 'source /edx/app/forum/ruby_env && source /edx/app/forum/devstack_forum_env && cd /edx/app/forum/cs_comments_service && bundle install && while true; do ruby app.rb -o 0.0.0.0 ; sleep 2; done'
Expand Down