[OldMongo FC-0004] ADR for migrate certificate signatures #31171

UvgenGen · 2022-10-19T11:52:13Z

Description

ADR about certificate signatures migration.

Other information

Remove the ability to read and write static assets to Old Mongo public-engineering#77
Migrate signature assets from MongoDB GridFS to the Credentials IDA credentials#1765

openedx-webhooks · 2022-10-19T11:52:16Z

Thanks for the pull request, @UvgenGen! Please note that it may take us up to several weeks or months to complete a review and merge your PR.

Feel free to add as much of the following information to the ticket as you can:

supporting documentation
Open edX discussion forum threads
timeline information ("this must be merged by XX date", and why that is)
partner information ("this is a course on edx.org")
any other information that can help Product understand the context for the PR

All technical communication about the code itself will be done via the GitHub pull request interface. As a reminder, our process documentation is here.

Please let us know once your PR is ready for our review and all tests are green.

ormsbee

I don't have a good grasp on what the data models and views are for this today, and whether the signature data canonically belongs in this app or in the Credentials service. I'd love to get more clarity on exactly what's going where as part of this ADR.

ormsbee · 2022-10-19T20:39:47Z

lms/djangoapps/certificates/docs/decisions/007-transfer-certificate-signatures-from-mongo.rst

@@ -0,0 +1,45 @@
+
+Transfer certificate signatures from Mongo


The title should specify where they're being transferred to.

Thanks, updated

ormsbee · 2022-10-19T20:40:39Z

lms/djangoapps/certificates/docs/decisions/007-transfer-certificate-signatures-from-mongo.rst

+
+Context
+*******
+As a part of Old Mongo Deprecation work https://github.com/openedx/public-engineering/issues/62


Please use the link syntax for rST here.

The links were changed

ormsbee · 2022-10-19T20:45:26Z

lms/djangoapps/certificates/docs/decisions/007-transfer-certificate-signatures-from-mongo.rst

+
+* credentials API must be extended to provide endpoints for adding and retrieving signatures
+
+  * `CourseCertificateViewSet` API


That's a viewset from the Credentials service, right? Is this going to involve the certificates app in edx-platform at all then?

Yes, that's a viewset from the Credentials service. For now we planning transfer only certificate signatures but get other certificate data from mongo.

ormsbee · 2022-10-19T21:04:49Z

lms/djangoapps/certificates/docs/decisions/007-transfer-certificate-signatures-from-mongo.rst

+
+  * `CourseCertificateViewSet` API
+
+* added signal to copy signatures from Mongo to Credentials on course publish, certificate or signature update


Suggested change

* added signal to copy signatures from Mongo to Credentials on course publish, certificate or signature update

* add a new `OpenEdxPublicSignal` that is emitted from Studio whenever certificate-related data is changed (e.g. certificate image upload, course import). The signature image can be exposed as a URL.

* signal receiver will copy the signature image to [????]

ormsbee · 2022-10-19T22:59:50Z

lms/djangoapps/certificates/docs/decisions/007-transfer-certificate-signatures-from-mongo.rst

+
+* added signal to copy signatures from Mongo to Credentials on course publish, certificate or signature update
+* added management command to copy signatures from (OLD or both?) Mongo to Credentials
+* updated certifecate views for getting signatures from Credentials


Suggested change

* updated certifecate views for getting signatures from Credentials

* update certificate views for getting signatures from Credentials

ormsbee · 2022-10-19T23:03:50Z

lms/djangoapps/certificates/docs/decisions/007-transfer-certificate-signatures-from-mongo.rst

+  * `CourseCertificateViewSet` API
+
+* added signal to copy signatures from Mongo to Credentials on course publish, certificate or signature update
+* added management command to copy signatures from (OLD or both?) Mongo to Credentials


I would strongly push for copying signatures out of both Old and Split Mongo. The deprecation of Old Mongo is prompting this work, but Credentials should have its own copy of this data since it's needed for serving certificate information. Also, it'd be weird to special case it for only Old Mongo courses (which are very rare), while having a different code path for Split.

@ormsbee Course certs are served out of LMS not Credentials, so are you proposing that we should serve Course Certs out of LMS but globally serve the signatures out of Credentials (for both course and program certs) prior to migrating the Course Certs themselves?

@hurtstotouchfire in the ADR we stated that signatures and certificate configurations will be copied to Credentials, but the signatures will be used only when signature from MongoDB is not available (right now this case is specific for Old Mongo courses). The certificate configurations will be pushed to credentials using the management command (planned only for Old Mongo courses), and based on new course certificate update signal (for Split courses).

In the future, we would like to move certificate rendering logic from LMS to Credentials+MFEs, as was proposed in BD-11.
I hope the intermediate step, when we need to get certificates configuration from MongoDB, but signatures from Credentials for Split courses, wouldn't be required.

@hurtstotouchfire We are proposing that the complete migration of course certificates is out of scope for this project. The driver here is to unblock the full deprecation of the Old Mongo Modulestore. Achieving full deprecation requires moving signature assets and we want to do it in a way aligned with the future vision for all certificates living in the Credentials IDA.

cdeery · 2022-11-01T16:30:07Z

lms/djangoapps/certificates/docs/decisions/007-transfer-certificate-signatures-from-mongo.rst

+
+Decision
+********
+To keep access to the certificate signatures will be used external signatures from credentials on WEB/PDF


There's something garbled in this sentence.

Thank you, we have updated it to be more specific

cdeery · 2022-11-01T16:31:01Z

lms/djangoapps/certificates/docs/decisions/007-transfer-certificate-signatures-from-mongo.rst

+
+* credentials API must be extended to provide endpoints for adding and retrieving signatures
+
+  * `CourseCertificateViewSet` API must be updated with possibility uploading signature


by "possibility", do you mean "optionally"?

That's right, the signature create/update should be optional when pushing a certificate configuration to Credentials from LMS

GlugovGrGlib · 2022-11-09T22:47:40Z

@ormsbee @cdeery
Please review the ADR once again, we have provided some updates to the unclear parts.

cdeery

I am adding my approval, but I would like to hear if @ormsbee has any additional feedback before it is merged.

GlugovGrGlib · 2022-11-29T22:18:58Z

@ormsbee I have changed the ADR's status to Accepted, squashed commits, rebased the changes to the latest master and force pushed them into working branch.
It is ready to be merged now.

ormsbee · 2022-11-30T14:53:49Z

Ack, sorry, forgot to merge this. But will do so once checks finish running.

openedx-webhooks · 2022-11-30T15:14:04Z

@UvgenGen 🎉 Your pull request was merged! Please take a moment to answer a two question survey so we can improve your experience in the future.

edx-pipeline-bot · 2022-11-30T18:45:24Z

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

edx-pipeline-bot · 2022-11-30T19:27:30Z

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

openedx-webhooks added the open-source-contribution PR author is not from Axim or 2U label Oct 19, 2022

UvgenGen force-pushed the sagirov/EDXOLDMNG-171 branch 2 times, most recently from 96140a6 to ace63fa Compare October 19, 2022 11:56

ormsbee reviewed Oct 19, 2022

View reviewed changes

UvgenGen force-pushed the sagirov/EDXOLDMNG-171 branch 2 times, most recently from b2a9309 to 211706b Compare October 24, 2022 10:19

cdeery reviewed Nov 1, 2022

View reviewed changes

UvgenGen force-pushed the sagirov/EDXOLDMNG-171 branch from 211706b to de7ae25 Compare November 4, 2022 10:17

hurtstotouchfire assigned cdeery Nov 8, 2022

GlugovGrGlib force-pushed the sagirov/EDXOLDMNG-171 branch from 8c5602a to 3c9550e Compare November 9, 2022 23:06

cdeery approved these changes Nov 10, 2022

View reviewed changes

ormsbee approved these changes Nov 17, 2022

View reviewed changes

docs: ADR for signatures migration

4434031

GlugovGrGlib force-pushed the sagirov/EDXOLDMNG-171 branch from 3c9550e to 4434031 Compare November 29, 2022 22:14

ormsbee merged commit 33968fb into openedx:master Nov 30, 2022

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[OldMongo FC-0004] ADR for migrate certificate signatures #31171

[OldMongo FC-0004] ADR for migrate certificate signatures #31171

UvgenGen commented Oct 19, 2022 •

edited

Loading

openedx-webhooks commented Oct 19, 2022

ormsbee left a comment

ormsbee Oct 19, 2022

GlugovGrGlib Nov 9, 2022

ormsbee Oct 19, 2022

GlugovGrGlib Nov 9, 2022

ormsbee Oct 19, 2022

UvgenGen Oct 20, 2022

ormsbee Oct 19, 2022

GlugovGrGlib Nov 9, 2022

ormsbee Oct 19, 2022

ormsbee Oct 19, 2022

hurtstotouchfire Nov 9, 2022

GlugovGrGlib Nov 9, 2022

e0d Nov 10, 2022

cdeery Nov 1, 2022

GlugovGrGlib Nov 9, 2022

cdeery Nov 1, 2022

GlugovGrGlib Nov 9, 2022

GlugovGrGlib Nov 9, 2022

GlugovGrGlib commented Nov 9, 2022

cdeery left a comment

GlugovGrGlib commented Nov 29, 2022

ormsbee commented Nov 30, 2022

openedx-webhooks commented Nov 30, 2022

edx-pipeline-bot commented Nov 30, 2022

edx-pipeline-bot commented Nov 30, 2022


		* credentials API must be extended to provide endpoints for adding and retrieving signatures

		* `CourseCertificateViewSet` API


		* `CourseCertificateViewSet` API

		* added signal to copy signatures from Mongo to Credentials on course publish, certificate or signature update

	* added signal to copy signatures from Mongo to Credentials on course publish, certificate or signature update
	* add a new `OpenEdxPublicSignal` that is emitted from Studio whenever certificate-related data is changed (e.g. certificate image upload, course import). The signature image can be exposed as a URL.
	* signal receiver will copy the signature image to [????]

	* updated certifecate views for getting signatures from Credentials
	* update certificate views for getting signatures from Credentials


		* credentials API must be extended to provide endpoints for adding and retrieving signatures

		* `CourseCertificateViewSet` API must be updated with possibility uploading signature

[OldMongo FC-0004] ADR for migrate certificate signatures #31171

[OldMongo FC-0004] ADR for migrate certificate signatures #31171

Conversation

UvgenGen commented Oct 19, 2022 • edited Loading

Description

Other information

openedx-webhooks commented Oct 19, 2022

ormsbee left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

GlugovGrGlib commented Nov 9, 2022

cdeery left a comment

Choose a reason for hiding this comment

GlugovGrGlib commented Nov 29, 2022

ormsbee commented Nov 30, 2022

openedx-webhooks commented Nov 30, 2022

edx-pipeline-bot commented Nov 30, 2022

edx-pipeline-bot commented Nov 30, 2022

UvgenGen commented Oct 19, 2022 •

edited

Loading