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

feat: Pin python3-saml again for duplicate attributes issue #32328

Merged
merged 1 commit into from
May 30, 2023

Conversation

timmc-edx
Copy link
Contributor

We're seeing an issue with repeated attributes causing auth failure for some IdPs, and need to take some time to investigate. See unpinning issue #32327 for details.

This is expected to reintroduce some bugs in testing SAML in devstack.

This has the effect of reverting the following PRs:

However, we're keeping the safe_lxml adjustments from the first PR, so it should be fine on that front to re-upgrade whenever we're ready.

We're seeing an issue with repeated attributes causing auth failure for
some IdPs, and need to take some time to investigate. See unpinning issue
#32327 for details.

This is expected to reintroduce some bugs in testing SAML in devstack.

This has the effect of reverting the following PRs:
- Unpinned: #32167 (partial)
- Upgraded: #32168

However, we're keeping the safe_lxml adjustments from the first PR, so it
should be fine on that front to re-upgrade whenever we're ready.
@@ -101,3 +101,11 @@ urllib3<2.0.0
# Sphinx==5.3.0 requires docutils<0.20
# Issue to unpin Sphinx to resolve this constraint: https://github.com/openedx/edx-lint/issues/338
docutils<0.20

# When we unpinned python3-saml and allowed it to upgrade from 1.9.0
Copy link
Contributor

Choose a reason for hiding this comment

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

great comment to leave here, thank you.

@matthugs
Copy link
Contributor

surfacing that there was concern about compatibility with the social-core library's version, but our current working hypothesis is that this version should work with current social-core. If there are further issues we'll need to revisit that hypothesis & maybe tweak versions to find a compatible pair, but for right now this is a worthy experiment for resolving the immediate issue.

@ilee2u
Copy link
Contributor

ilee2u commented May 30, 2023

Context for this PR can be found in this ticket: MST-1916.

Copy link
Contributor

@schenedx schenedx left a comment

Choose a reason for hiding this comment

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

👍 I double checked that python3-saml version 1.9 is what we want to revert back to.

@timmc-edx timmc-edx merged commit cc9ec69 into master May 30, 2023
@timmc-edx timmc-edx deleted the timmc/downgrade-python3-saml branch May 30, 2023 17:22
@edx-pipeline-bot
Copy link
Contributor

EdX Release Notice: This PR has been deployed to the staging environment in preparation for a release to production.

@edx-pipeline-bot
Copy link
Contributor

EdX Release Notice: This PR has been deployed to the production environment.

Yagnesh1998 pushed a commit to ManpraXSoftware/edx-platform that referenced this pull request Jun 8, 2023
…32328)

We're seeing an issue with repeated attributes causing auth failure for
some IdPs, and need to take some time to investigate. See unpinning issue
openedx#32327 for details.

This is expected to reintroduce some bugs in testing SAML in devstack.

This has the effect of reverting the following PRs:
- Unpinned: openedx#32167 (partial)
- Upgraded: openedx#32168

However, we're keeping the safe_lxml adjustments from the first PR, so it
should be fine on that front to re-upgrade whenever we're ready.
Yagnesh1998 pushed a commit to ManpraXSoftware/edx-platform that referenced this pull request Jun 8, 2023
…32328)

We're seeing an issue with repeated attributes causing auth failure for
some IdPs, and need to take some time to investigate. See unpinning issue
openedx#32327 for details.

This is expected to reintroduce some bugs in testing SAML in devstack.

This has the effect of reverting the following PRs:
- Unpinned: openedx#32167 (partial)
- Upgraded: openedx#32168

However, we're keeping the safe_lxml adjustments from the first PR, so it
should be fine on that front to re-upgrade whenever we're ready.
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

Successfully merging this pull request may close these issues.

6 participants