Skip to content
This repository was archived by the owner on Jan 22, 2025. It is now read-only.

fix: update EXDNOTES_INTERNAL_API port - #1

Open
erikmartus wants to merge 1 commit into
masterfrom
fix-internal-api
Open

fix: update EXDNOTES_INTERNAL_API port#1
erikmartus wants to merge 1 commit into
masterfrom
fix-internal-api

Conversation

@erikmartus

Copy link
Copy Markdown
Collaborator

According to the docker file this service is listening on port 8120, but LMS cannot find service when attempting to access the Notes page within a course. Meanwhile, notes can be successfully created and fetched on unit pages since it is using the public API. Changing the port from 8000 to 8120 for the internal API allows for the Notes page to work correctly.

@jcohen28

Copy link
Copy Markdown
Member

@erikmartus you probably haven't received any attention on this pr because it is an internal pr to merge against our fork's master rather than the parent master. Probably for the best, since we have to clean it up first as we discussed earlier this week.

# Student notes
EDXNOTES_CLIENT_NAME = "notes"
EDXNOTES_INTERNAL_API = "http://notes:8000/api/v1"
EDXNOTES_INTERNAL_API = "http://notes:8120/api/v1"

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

This setting looks like it is shared between production and development. However, if I am reading the other configurations correctly, production does use port 8000 while only development uses 8120. If that is true, then modifying in place like this would fix development but at the price of breaking production. Instead, we might need to move this setting into the other files called openedx-lms-development-settings and openedx-lms-production-settings so the two environments can have distinct values.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Once you make that change. Please first test that development works. Then please work with @edazzocaisser to deploy the branch to our production environment and test that it works before opening a pr to the parent repo.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants