Improve Migration CLI and Exception classes #34
Conversation
src/Migration/Exception.php
Outdated
| public string $resourceType; | ||
|
|
||
| public string $resourceId; | ||
|
|
||
| public function __construct(string $resourceType, string $message, int $code = 0, ?\Throwable $previous = null, string $resourceId = '') | ||
| public function __construct(string $resourceName, string $resourceType, string $message, int $code = 0, ?\Throwable $previous = null, string $resourceId = '') |
There was a problem hiding this comment.
Not sure about name of variables. If one is Storage and one is Bucket, I think we should do $service and $resource
There was a problem hiding this comment.
Migration lib is supposed to be agnostic so it's all called groups throughout the library, changing it here would mean we would have to change it everywhere. We could probably change the terminology for G2 but I think we should keep it the same for G1
src/Migration/Sources/Appwrite.php
Outdated
| new Exception( | ||
| Resource::TYPE_DATABASE, | ||
| Transfer::GROUP_DATABASES, | ||
| $e->getMessage() | ||
| ) |
There was a problem hiding this comment.
We should pass $e as previous everywhere we can, that way the stack trace will include the trace from the original exception too
There was a problem hiding this comment.
Since we're passing $e we could get rid for code and message and just defer it off previous, wdyt?
There was a problem hiding this comment.
Better to keep them separate; we might not want to defer every message/code to previous
This PR pushes the exception resource name into the exception class so we can access it easier in our analytics and migration CLI