[Feature] Overwrite test by URI during QTI import #2491
Conversation
Front-end summary Node 18
|
shpran
changed the title
| } | ||
| } catch (\common_exception_RestApi $e) { | ||
| } catch (common_exception_RestApi $e) { | ||
| return $this->returnFailure($e); |
There was a problem hiding this comment.
@shpran would it be asking too much if we could make sure that the exceptions caught here have logs with:
- Exception class name that was triggered
- Exception message
- Exception trace
Maybe we already have something on returnFailure implementation?
I am asking, cause generally this logs with only exception message do not help at all as we have no idea where it happened. This is a critical process for the whole ingestion process, so I think it is useful to have this logs
Wdyt?
There was a problem hiding this comment.
This one doesn't relate to my changes (just fixed FQN), but sure I will improve these logs 😎
Thanks for suggestion
There was a problem hiding this comment.
@shpran I know this is not part of your changes, but today is a bit of a pain to track errors when we are generating translations. Could you please add logs for that?
There was a problem hiding this comment.
I checked returnFailuer implementation and found, that we use Advanced Logger feature there, so message, file, line and trace should be logged:
…tqti into feat/pisa25-744/overwrite-test-by-uri
There was a problem hiding this comment.
Thanks @shpran
- New code is covered by tests (if applicable)
- Tests are running successfully (old and new ones) on my local machine (if applicable)
- New code is respecting code style rules
- New code is respecting best practices
- New code is not subject to concurrency issues (if applicable)
- Feature is working correctly on my local machine (if applicable)
- Acceptance criteria are respected
- Pull request title and description are meaningful
Version
There are 0 BREAKING CHANGE, 1 feature, 0 fix |
PISA25-774
Changes
overwriteTestreplaced byoverwriteTestUrioverwriteTestUrirequest parameterRelated PRs