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

Documenting buggy behaviour while dynamic config #86

Closed
Tracked by #539 ...
ghassanmas opened this issue Nov 15, 2022 · 2 comments
Closed
Tracked by #539 ...

Documenting buggy behaviour while dynamic config #86

ghassanmas opened this issue Nov 15, 2022 · 2 comments
Assignees
Milestone

Comments

@ghassanmas
Copy link
Member

ghassanmas commented Nov 15, 2022

Problematic configs

  1. ACCESS_TOKEN_COOKIE_NAME
    When this doesn't exists: ACCESS_TOKEN_COOKIE_NAME:"edx-jwt-cookie-header-payload"

      js
      lib.js:151 Uncaught Error: getLocale called before configuring i18n. Call configure with messages first.
          at E (lib.js:151:11)
          at p (ErrorPage.jsx:26:40)
          at Va (react-dom.production.min.js:153:146)
          at bu (react-dom.production.min.js:261:496)
          at ds (react-dom.production.min.js:246:265)
          at As (react-dom.production.min.js:246:194)
          at ns (react-dom.production.min.js:239:172)
          at Ju (react-dom.production.min.js:230:137)
          at Ls (react-dom.production.min.js:281:43)
          at react-dom.production.min.js:284:301
    

This occurs on frontend-app-learning when being authenticated, it will not occur if you browsing a course as anonymous.

  1. STUDIO_BASE_URL I found that course authoring doesn't take this key into consideration, though it makes call to mfe config, but nonetheless it doesn't affect.

Solution Proposal

  1. The solution of one can be ensure that value is should be set at build time and if operator want to change they need to rebuild the image
  2. Create a proxy role where wet STUDIO_BASE_URL as cms-api and then create a proxy role simliar to the one for config api that proxy cms-api to => STUDIO_BASE_URL. One thing I am not sure about this approach is that how it would work with k8s, but in any case it uses the same principle of the how we proxy the config api.

On a more general point, I think it would better to just create two proxy role, for the LMS and CMS such that always set a predifiend token for of both and then add proxy to lms/cms, instead of just adding proxy for dynamic config uri we set it at the sub/domain level.

  1. [Edit] I found the cause while it's picking it, and suggest a fix at fix: force studio url to reload if changed openedx/frontend-app-authoring#389, though I still see good value in the above.
@ghassanmas ghassanmas changed the title Documeting buggy behavour while dynamic config Documeting buggy behaviour while dynamic config Nov 15, 2022
ghassanmas added a commit to ghassanmas/frontend-app-course-authoring that referenced this issue Nov 15, 2022
  This chagne make it possible if this module was loaded **then**
  the configuration for studio url is changed, then it will pick
  the last value.

  More context overhangio/tutor-mfe/issues/86
@arbrandes arbrandes added this to the Olive Release Candidate milestone Nov 16, 2022
@arbrandes
Copy link
Collaborator

Thanks for investigating this, @ghassanmas.

I think it would better to just create two proxy role, for the LMS and CMS such that always set a predifiend token for of bot

This would solve the URL problem, but I'd be concerned about scalability. This adds adds a little processing time to each request that hits Caddy, and we're talking about many more requests than just the initial configuration one. This could conceivably introduce a bottleneck to the deployment.

And yes, there's the k8s question we'd have to look into.

ghassanmas added a commit to ghassanmas/frontend-app-course-authoring that referenced this issue Nov 17, 2022
  This chagne make it possible if this module was loaded **then**
  the configuration for studio url is changed, then it will pick
  the last value.

  More context overhangio/tutor-mfe/issues/86
@regisb regisb changed the title Documeting buggy behaviour while dynamic config Documenting buggy behaviour while dynamic config Nov 17, 2022
arbrandes pushed a commit to openedx/frontend-app-authoring that referenced this issue Nov 18, 2022
  This chagne make it possible if this module was loaded **then**
  the configuration for studio url is changed, then it will pick
  the last value.

  More context overhangio/tutor-mfe/issues/86
@arbrandes arbrandes moved this to In progress in Frontend Working Group Nov 21, 2022
ghassanmas added a commit to ghassanmas/frontend-app-gradebook that referenced this issue Nov 29, 2022
  This change make it possible if LMS url to be changed, that
  the last value will be picked.

  This is simlair openedx/frontend-app-authoring/pull/389
  which issue overhangio/tutor-mfe/issues/86, the fixes is needed
  so that dynamic config would work with tutor:
  overhangio/tutor-mfe/pull/69
ghassanmas added a commit to ghassanmas/frontend-app-gradebook that referenced this issue Nov 29, 2022
  This change make it possible if LMS url to be changed, that
  the last value will be picked.

  This is simlair openedx/frontend-app-authoring/pull/389
  which issue overhangio/tutor-mfe/issues/86, the fixes is needed
  so that dynamic config would work with tutor:
  overhangio/tutor-mfe/pull/69
ghassanmas added a commit to ghassanmas/frontend-app-gradebook that referenced this issue Nov 29, 2022
  This change make it possible if LMS url to be changed, that
  the last value will be picked.

  This is simlair openedx/frontend-app-authoring/pull/389
  which issue overhangio/tutor-mfe/issues/86, the fixes is needed
  so that dynamic config would work with tutor:
  overhangio/tutor-mfe/pull/69
ghassanmas added a commit to ghassanmas/frontend-app-gradebook that referenced this issue Nov 29, 2022
  This change make it possible if LMS url to be changed, that
  the last value will be picked.

  This is simlair openedx/frontend-app-authoring/pull/389
  which issue overhangio/tutor-mfe/issues/86, the fixes is needed
  so that dynamic config would work with tutor:
  overhangio/tutor-mfe/pull/69
@arbrandes
Copy link
Collaborator

@ghassanmas, I'm closing this in favor of opening new issues for specific problems after #69 is merged.

ghassanmas added a commit to ghassanmas/frontend-app-gradebook that referenced this issue Dec 6, 2022
  This change make it possible if LMS url to be changed, that
  the last value will be picked.

  This is simlair openedx/frontend-app-authoring/pull/389
  which issue overhangio/tutor-mfe/issues/86, the fixes is needed
  so that dynamic config would work with tutor:
  overhangio/tutor-mfe/pull/69
arbrandes pushed a commit to openedx/frontend-app-gradebook that referenced this issue Dec 8, 2022
  This change make it possible if LMS url to be changed, that
  the last value will be picked.

  This is simlair openedx/frontend-app-authoring/pull/389
  which issue overhangio/tutor-mfe/issues/86, the fixes is needed
  so that dynamic config would work with tutor:
  overhangio/tutor-mfe/pull/69
arbrandes pushed a commit to openedx/frontend-app-gradebook that referenced this issue Dec 8, 2022
  This change make it possible if LMS url to be changed, that
  the last value will be picked.

  This is simlair openedx/frontend-app-authoring/pull/389
  which issue overhangio/tutor-mfe/issues/86, the fixes is needed
  so that dynamic config would work with tutor:
  overhangio/tutor-mfe/pull/69
ghassanmas added a commit to ghassanmas/frontend-app-course-authoring that referenced this issue Dec 11, 2022
  This chagne make it possible if this module was loaded **then**
  the configuration for studio url is changed, then it will pick
  the last value.

  More context overhangio/tutor-mfe/issues/86
arbrandes pushed a commit to openedx/frontend-app-authoring that referenced this issue Dec 12, 2022
  This chagne make it possible if this module was loaded **then**
  the configuration for studio url is changed, then it will pick
  the last value.

  More context overhangio/tutor-mfe/issues/86
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