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: Don't build uwsgi with xml support. #1157

Merged
merged 2 commits into from
Nov 15, 2024

Conversation

feanil
Copy link
Contributor

@feanil feanil commented Nov 12, 2024

When uwsgi runs with xml support, it throws the following error on startup with the latest version of edx-platform:

xmlsec.InternalError: (-1, 'lxml & xmlsec libxml2 library version mismatch')

See xmlsec/python-xmlsec#320 for more details on this.

Essentially, the uWSGI wheel is built against a different version of libxml2 and it causes this version mismach error when trying to load xmlsec. The xml support in uWSGI is only needed for runing with xml configuration files, which tutor does not do. Following the guidance of the above issue, I updated the openedx Dockerfile to no longer compile with any xml support as a part of the build. The time to build uWSGI was only slightly more than building from cache so I'm not concerned about major slowdowns in the build time for un-cached builds.

@DawoudSheraz DawoudSheraz changed the base branch from master to nightly November 13, 2024 05:41
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.

This seems to be one of the first ripple effects of the pending uwsgi deprecation... 😨

Since this is fixing a bug, can you please add a changelog entry? https://docs.tutor.edly.io/tutor.html#contributing

@DawoudSheraz
Copy link
Contributor

Since ubuntu PR was merged in nightly, this PR should be merged to nightly as well. Apart from changelog, you might want to rebase the branch on nightly.

When uwsgi runs with xml support, it throws the following error on
startup with the latest version of edx-platform:

```
xmlsec.InternalError: (-1, 'lxml & xmlsec libxml2 library version mismatch')
```

See xmlsec/python-xmlsec#320 for more details
on this.

Essentially, the uWSGI wheel is built against a different version of
libxml2 and it causes this version mismach error when trying to load
xmlsec.  The xml support in uWSGI is only needed for runing with xml
configuration files, which tutor does not do.  Following the guidance of
the above issue, I updated the `openedx` Dockerfile to no longer compile
with any xml support as a part of the build. The time to build uWSGI was
only slightly more than building from cache so I'm not concerned about
major slowdowns in the build time for un-cached builds.
@feanil feanil force-pushed the feanil/fix_uwsgi_xml_issues_master branch from f1edbd9 to 8aba298 Compare November 13, 2024 18:17
@feanil
Copy link
Contributor Author

feanil commented Nov 13, 2024

Added a changelog entry, let me know if you need anything else.

@DawoudSheraz DawoudSheraz requested a review from regisb November 14, 2024 05:48
@DawoudSheraz
Copy link
Contributor

@regisb Can you take another look at this, please? Thanks

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.

lgtm! we should now split the django-redis and uwsgi installations, because they use different installation instructions, but we can do this in a separate PR.

@regisb regisb merged commit 437e2cd into overhangio:nightly Nov 15, 2024
2 checks passed
@feanil feanil deleted the feanil/fix_uwsgi_xml_issues_master branch November 15, 2024 14:43
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.

3 participants