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

🐛Skip category when error occurred during components parsing #123

Merged
merged 4 commits into from
Mar 24, 2022

Conversation

AdrySky
Copy link
Contributor

@AdrySky AdrySky commented Mar 23, 2022

Description

This will fix the components not rendering in the sidebar when error occurred in the script. It'll skip the category where the error occurred but still render every other components.

Pull Request Type

  • Xircuits Core (Jupyterlab Related changes)
  • Xircuits Canvas (Custom RD Related changes)
  • Xircuits Component Library
  • Testing Automation
  • Documentation
  • Sidebar

Type of Change

  • New feature (non-breaking change which adds functionality)
  • Bug fix (non-breaking change which fixes an issue)
  • Breaking change (fix or feature that would cause existing functionality to not work as expected)
  • This change requires a documentation update

Tests

  1. Open any component's script
  2. Make an error(e.g x = x else 5)
  3. Try refreshing the components list
  4. It'll display what the error
  5. Also, the category of the error occurred will not render

Tested on?

  • Windows
  • Linux Ubuntu
  • Centos
  • Mac
  • Others (State here -> xxx )

Notes

Currently, it'll only detect one error in the same file. It can't detect multiple error in a different folder/file.

@AdrySky AdrySky added the bug Something isn't working label Mar 23, 2022
@AdrySky AdrySky requested a review from MFA-X-AI March 23, 2022 02:08
@AdrySky AdrySky self-assigned this Mar 23, 2022
Copy link
Member

@MFA-X-AI MFA-X-AI left a comment

Choose a reason for hiding this comment

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

Awesome, this will really help in troubleshooting errors when the python parser breaks down.

xircuits error catch

Two things I'd like to comment on though -

  1. I noticed that the notification overlays each other, ie if you leave the Xircuits running with the error log active, it will create multiple notifications that you need to press x multiple times. The best solution is just not to send it again if the notification panel is active. Another approach that would also has to do with the 2nd thing which is
  2. The frequency of how the sidebar is refreshed. When fixing the syntax error code, the notification would pop-up every 3 seconds or so, which isn't really a good user experience. I think the best way of doing this is, if the python parser breaks down slow down the refresh rate to 5 mins, or until the user presses refresh.

Since it's a bit disruptive, I can't merge this yet. Hope you can look into it thanks.

@AdrySky
Copy link
Contributor Author

AdrySky commented Mar 23, 2022

Thanks for reviewing. It weird that your list get refreshed every 3 seconds as it should only refresh every 10 min. So I just make it don't refresh at all when an error occurred. However, you can still refresh list by pressing refresh button.

Edit: Ok my mistake..it does refresh every 3 seconds. I forgot to open xircuits. Ignore my previous commit, the bug still persist.

@AdrySky AdrySky requested a review from MFA-X-AI March 24, 2022 02:51
Copy link
Member

@MFA-X-AI MFA-X-AI left a comment

Choose a reason for hiding this comment

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

Awesome. Now it warns the user when there's a syntax error, but not too disruptive to the point where I can actually still continue testing the things. Good work!

@MFA-X-AI MFA-X-AI merged commit 1fd6965 into master Mar 24, 2022
@MFA-X-AI MFA-X-AI deleted the adry/fix-component-parser branch March 24, 2022 03:22
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants