Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

SG-36129 add handling for data processing errors #158

Closed

Conversation

pscadding
Copy link
Contributor

This was added to aid a feature on the tk-multi-shotgunpanel where if a bad custom sort field was added it would result in a error to the console but nothing in the UI.

This change means that the data_refresh_fail signal will be fired if the fetched data can't be processed.

I have concerns about the impact this change may have on other tools using this framework.

Copy link
Contributor

@carlos-villavicencio-adsk carlos-villavicencio-adsk left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Since we didn't have error handling on this logic before, would it make sense to make the fail silent? I mean, just log everything and let it pass? As you mentioned in your other PR, we're not sure if this can impact other dependent modules.

@pscadding
Copy link
Contributor Author

Having done more testing and discussed with @carlos-villavicencio-adsk I'm going to close this PR without merging.
We would prefer not to add risk by changing the utils here for the sake catching what will likely be a edge case scenario with the configuration of the panel app.

I still think the code change here is probably the right approach, had we chosen to go with it, so if needed in the future we could implement something similar.

@pscadding pscadding closed this Aug 30, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants