And/generate certificate based on rten request #187
Conversation
|
|
||
| if response.status_code == status.HTTP_200_OK and getattr(settings, "ENABLE_CERTIFICATE_PUBLISHER", False): | ||
| result_notification = ResultNotificationBackend(request_data=request.data) | ||
| result_notification.run_pipeline() |
There was a problem hiding this comment.
This pipeline is run in sync. Why not async?
There was a problem hiding this comment.
If fails, the record would not be created in rten model
There was a problem hiding this comment.
Let me answer with a question, Why async ?
There was a problem hiding this comment.
If fails, the record would not be created in rten model
yes, probably the generics drf views implement an atomic block, but the result will be stored if that fails due to pearson exceptions that are handled by the ErrorRealTimeImportHandler class
There was a problem hiding this comment.
Let me answer with a question, Why async ?
Because in that case you isolate the RTEN request process from the generated certificate process.
You could avoid some 500 error codes, and maybe the client(Pearson) would think why we are getting 500...
All code related to the pipeline running in sync could make the request slower or fails and I think the Pearson Client is not aware that it is not related with the purpose of data sent...
There was a problem hiding this comment.
Errors should never pass silently. Python zen
To avoid errors I can use a simple try except but I want that the client get a 500 error it's the only way to know that something is happening
request slower, yes, every line that you add in code affects the time response and when the code that you add is too much or needs a lot of time to be executed , one option is an async task, so is this that case ? have you experimented a slow response ? or have you identified a pipeline step that needs a lot of time ?
There was a problem hiding this comment.
errors would happen in the sync or async context. Anyway sentry would report both envs. So it wouldn't pass silently meanwhile sentry is working xD.
The point is who reports the error: the worker or the lms pod. And also who is being affected by that...
Anyway, if you consider is ok I could accept that.
There was a problem hiding this comment.
The issue isn't about what reports the errors; it's about who receives them. On one hand, we have the client using the service, and on the other, we have the Sentry service that goes unchecked until the client complains.
Finally, t main purpose of the async tasks is to execute heavy processes, not to avoid exceptions. For that, you can catch the exceptions and use a simple log.error if you want to report it to Sentry.
eox_nelp/pearson_vue/pipeline.py
Outdated
| dict: A dictionary containing extracted data including exam result, client authorization ID, | ||
| enrollment ID, anonymous user model ID, and anonymous user ID. | ||
| """ | ||
| client_authorization_id = request_data["authorization"]["clientAuthorizationID"] |
There was a problem hiding this comment.
Are you going to handle key error with pearsonvue exceptions?? or could be other PR
There was a problem hiding this comment.
Thanks for the reminder, I will implement that in this PR
|
|
||
| Returns: | ||
| dict: A dictionary containing the enrollment object if found, otherwise an empty dictionary. | ||
| """ |
There was a problem hiding this comment.
Could you add the block code:
if not enrollment:In order to not be concerned it is first
get_enrollment_from_anonymous_user_id or get_enrollment_from_id
Description
This implements a new backend in the result notification view with new pipeline steps that allow to create a new certificate
Testing instructions
ENABLE_CERTIFICATE_PUBLISHER, this will enable the result notification pipeline and the external certificate publisher, its not necessary to test the external call so you can use an invalid mode, or if you want to test the external call setCERTIFICATE_PUBLISHER_VALID_MODESwith the desired mode{ "eventType":"RESULT_AVAILABLE", "eventTime":"?", "candidate":{ "candidateName":{ "firstName":"Maximo", "lastName":"Tercero" }, "email":"?", "lastUpdate":"?", "vueCandidateID":"?", "clientCandidateID":"?" }, "attempt":"$", "authorization":{ "clientAuthorizationID": "1000-3000" }, "exams":{ "exam":[ { "examDefinition":{ "examSeriesCode":"?", "examLanguageCode":"?", "examName":"?", "deliveryModel":"?", "vueExamVersionId":"?", "vueExamRevisionID":"?", "clientExamVersionId":"?", "examForm":"?", "isBetaVersion":"#" }, "examResult":{ "outcomeType":"?", "startTime":"?", "timeUsed":"$", "passingScore":"0.8", "score":"0.832", "rawScore":"$", "grade":"0.5", "correct":"$", "incorrect":"$", "skipped":"$", "unscored":"$" }, "noShow":"#", "expired":"#", "ndaRefused":"#", "deliveryEventType":"?" } ] }, "vueAppointmentID":"$", "clientCode":"?", "vueCandidateID":"$", "clientCandidateID":"?" }/admin/certificates/generatedcertificateCERTIFICATE_PUBLISHER_VALID_MODESa message with the following content must appearThe %s certificate associated with the user <%s> and course <%s> doesn't have a valid mode and therefore its data won't be published."Note
The whole integration can be tested by following this guide