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

OEP-32 has been superseded and should be updated #407

Open
e0d opened this issue Nov 4, 2022 · 7 comments
Open

OEP-32 has been superseded and should be updated #407

e0d opened this issue Nov 4, 2022 · 7 comments
Labels
help wanted Ready to be picked up by anyone in the community under discussion

Comments

@e0d
Copy link
Contributor

e0d commented Nov 4, 2022

The guidance for external ids in OEP-32 is currently incorrect and that OEP has been superseded by this ADR.

The OEP needs a significant rework.

@e0d e0d mentioned this issue Nov 4, 2022
@robrap
Copy link
Contributor

robrap commented Nov 4, 2022

Do you think the guidance is still correct for ids internal to the platform, and this is just about correcting guidance for external ids?

@e0d
Copy link
Contributor Author

e0d commented Nov 4, 2022

The ADR only covers the case of external IDs. So, job one would be correcting that.

My preference would be that auto increment database IDs were not used as identifiers, but that's a much bigger, breaking, change I believe.

@robrap
Copy link
Contributor

robrap commented Nov 5, 2022

I agree with your preference, but the point of the OEP was to codify what to use, given the reality of our situation. Better than using user id in some cases, and username in other cases, as was happening.

@sarina
Copy link
Contributor

sarina commented May 17, 2024

@robrap @e0d - is OEP-32 completely incorrect and should be marked as Obsolete, or is it simply that parts need updating?

@sarina sarina added the waiting on author PR author needs to resolve review requests, answer questions, fix tests, etc. label May 17, 2024
@robrap
Copy link
Contributor

robrap commented May 17, 2024

It is partially correct. It is valid for internal ids, and invalid for external ids. I'm not sure if there are legacy exceptions to the external ids case that do use this same internal id.

@sarina
Copy link
Contributor

sarina commented May 20, 2024

Thanks. Assuming #586 passes, I will mark this OEP as Needs Revision and point to this thread.

@sarina sarina added help wanted Ready to be picked up by anyone in the community and removed waiting on author PR author needs to resolve review requests, answer questions, fix tests, etc. labels May 20, 2024
sarina added a commit that referenced this issue Jul 4, 2024
@sarina
Copy link
Contributor

sarina commented Jul 4, 2024

Put OEP-32 in Needs Revision status, please take a look at #603 - this ticket will be kept open for discussion on how to actually update the OEP for real

@sarina sarina added under discussion help wanted Ready to be picked up by anyone in the community and removed help wanted Ready to be picked up by anyone in the community labels Jul 4, 2024
sarina added a commit that referenced this issue Jul 5, 2024
sarina added a commit that referenced this issue Jul 18, 2024
davidjoy pushed a commit to davidjoy/open-edx-proposals that referenced this issue Aug 28, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
help wanted Ready to be picked up by anyone in the community under discussion
Projects
None yet
Development

No branches or pull requests

3 participants