-
-
Notifications
You must be signed in to change notification settings - Fork 489
Refactor Init process to use new CLI gRPC API #342
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
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@silvanocerza what happens if the initialized fails or rejects? is it ok to proceed calling coreClient()?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think it's fine, if the Init gRPC function fails it means it has been called, so Create was successful meaning we have an internal instance in the CLI daemon and we have a "working" client.
If Init fails completely though we must have some other huge problems, with the refactoring failure to load a platform or library just logs an error, but it doesn't stop completely.
I still don't like how I implemented this though, I'd like to show the user a notification when there are this kind of failures but am not sure how to handle that.
e6e139d to
f73f321
Compare
ubidefeo
commented
May 18, 2021
@silvanocerza
Installing a platform rebuilds the tools>board menu, but uninstalling does not
- install a platform
- look at the boards menu: it's there
- uninstall the platform
- look at the boards menu: it's still there
@ubidefeo I investigated a bit, the same issue is also present when uninstalling a library.
It seems to me this is a race condition, the list of boards divided by platform is retrieved before the platform is uninstalled, the strange thing though is that after installing a new one I'd expect it to stop showing the uninstalled platform but it doesn't. 🤔
This requires some further investigation I guess.
b6b1073 to
178a11d
Compare
Rebased to fix conflicts and trigger a new build after updating the version of the Arduino CLI.
The issues with core/platforms uninstallation is solved, it was an issue in the CLI after all.
be4546b to
178a11d
Compare
178a11d to
13de2f8
Compare
@ubidefeo
ubidefeo
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
been poking around for almost an hour and haven't found anything which isn't already an issue in the Beta 7.
LGTM
This has been superseded by #506. Closing.
The CLI initialization process has been changed in significant ways with PR arduino/arduino-cli#1274, it needs to be thorougly tested before merging to avoid breaking the IDE.