chore: update the DryRunService to improve error messages #4001
Conversation
…le scripts with the same name are found across integrations. - Added logic to differentiate between variants within the same integration and scripts across different integrations, prompting users to specify the correct variant or integration ID as needed.
|
Your mrge subscription is currently inactive. Please reactivate your subscription to receive AI reviews and use mrge. |
![propel-code-bot[bot]](https://avatars.githubusercontent.com/in/1179004?s=60&v=4){kind=small}
| scriptInfo = script; | ||
| foundInProviderConfigKey = providerConfigKey; | ||
| providerConfigKey = integration.providerConfigKey; |
There was a problem hiding this comment.
[DataTypeCheck]
There's a potential issue with the assignment of providerConfigKey. You're assigning providerConfigKey to foundInProviderConfigKey and then reassigning providerConfigKey immediately after. This could lead to both variables having the same value by the next iteration, which might cause incorrect comparisons in the if (foundInProviderConfigKey === providerConfigKey) condition.
Consider storing the original value before reassigning:
scriptInfo = script;
foundInProviderConfigKey = providerConfigKey;
providerConfigKey = integration.providerConfigKey;
| `Script "${syncName}" exists in multiple integrations (${foundInProviderConfigKey}, ${providerConfigKey}). Please specify which integration to use with "--integration-id"` | ||
| ) | ||
| ); | ||
| } |
There was a problem hiding this comment.
I am not sure I follow the logic. I don't think you can detect if variants exist unless querying the API
There was a problem hiding this comment.
Yeah, wasn't too sure about this one.
if we cant distinguish between the two should it just be an update to the existing error message to include variant?
Multiple integrations contain a script named "${syncName}". Please use "--integration-id or --variant to specify"
There was a problem hiding this comment.
Maybe I don't really understand what the problem is with variants. Based on the code above it looks like if a variant is not specified the CLI default to the base one so I am not sure I understand the issue
There was a problem hiding this comment.
Let me dig into this further get what the root cause of this request was
TBonnin
force-pushed
the
master
branch
3 times, most recently
from
f519537 to
87a5092
Compare
EXT-610
Open Issues
Will require validation and additional testing for variants
This PR enhances error messaging in the CLI's DryRunService by differentiating between script variants within the same integration versus scripts across different integrations. When a script name conflict is detected, users now receive more specific guidance on whether to use '--variant' or '--integration-id' flags.
This summary was automatically generated by @propel-code-bot