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

fix(tutorecommerce): add paymentprocessors.json to k8s deployments an… #55

Merged
merged 3 commits into from
Dec 9, 2023

Conversation

hoffmannkrzysztof
Copy link
Contributor

…d kustomization-configmapgenerator

Mounting volume with paymentprocessors.json is missing while using k8s stack.

@regisb
Copy link
Contributor

regisb commented Dec 6, 2023

Hi! Thanks for your PR. You're right that we need to bind-mount not just the production.py setting in the ecommerce container. But I believe we should actually mount all files from the settings/ directory. Thus, we should write something like:

- name: ecommerce-settings
  files:{% for file in "plugins/ecommerce/apps/ecommerce/settings/"|walk_templates %}
  - {{ file }}{% endfor %}

And then, in the ecommerce container, we should mount the entire directory:

volumeMounts:
            - mountPath: /openedx/ecommerce/ecommerce/settings/
              name: settings
volumes:
        - name: settings
          configMap:
            name: ecommerce-settings

(I didn't test this configuration and I'm not sure it's 100% correct, but I think you get the point, right?)

See for reference the "openedx-settings-lms" config for the LMS. Can you please update your PR to reflect that?

@regisb regisb requested a review from Faraz32123 December 6, 2023 09:52
@hoffmannkrzysztof
Copy link
Contributor Author

@regisb thanks for the tip. I implemented fix

Copy link
Contributor

@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.

Nice! Can you please add a changelog entry with scriv create?

@regisb regisb merged commit 1a2d7f9 into overhangio:master Dec 9, 2023
1 check passed
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.

2 participants