-
-
Notifications
You must be signed in to change notification settings - Fork 422
[breaking] core update-index do not stop at the first download error
#1866
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
[breaking] core update-index do not stop at the first download error
#1866
Conversation
6670d50 to
732bdc5
Compare
Codecov Report
@@ Coverage Diff @@ ## master #1866 +/- ## ========================================== + Coverage 36.62% 36.65% +0.02% ========================================== Files 231 231 Lines 19610 19655 +45 ========================================== + Hits 7182 7204 +22 - Misses 11600 11622 +22 - Partials 828 829 +1
Flags with carried forward coverage won't be shown. Click here to find out more.
Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here. |
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.
Describe the problem
The value of the cc.arduino.cli.commands.v1.ArduinoCoreService.DownloadResult.successful field is set to true when a download fails.
To reproduce
Set up
$ arduino-cli version
arduino-cli.exe Version: git-snapshot Commit: 732bdc54 Date: 2022年09月07日T09:59:11Z
$ export ARDUINO_BOARD_MANAGER_ADDITIONAL_URLS=http://example.com/package_foo_index.json
$ arduino-cli daemon
Daemon is now listening on 127.0.0.1:50051
Demo
Use grpcurl to run the following commands in another terminal.
$ grpcurl \
-plaintext \
-import-path ./rpc \
-proto cc/arduino/cli/commands/v1/commands.proto \
127.0.0.1:50051 \
cc.arduino.cli.commands.v1.ArduinoCoreService.Create
{
"instance": {
"id": 1
}
}
$ grpcurl \
-plaintext \
-import-path ./rpc \
-proto cc/arduino/cli/commands/v1/commands.proto \
-d '{"instance": {"id": 1}}' \
127.0.0.1:50051 \
cc.arduino.cli.commands.v1.ArduinoCoreService.Init
{
"error": {
"code": 9,
"message": "Loading index file: loading json index file C:\\Users\\per\\AppData\\Local\\Arduino15\\package_foo_index.json: open C:\\Users\\per\\AppData\\Local\\Arduino15\\package_foo_index.json: The system cannot find the file specified."
}
}
$ grpcurl \
-plaintext \
-import-path ./rpc \
-proto cc/arduino/cli/commands/v1/commands.proto \
-d '{"instance": {"id": 1}}' \
127.0.0.1:50051 \
cc.arduino.cli.commands.v1.ArduinoCoreService.UpdateIndex
{
"downloadProgress": {
"url": "https://downloads.arduino.cc/packages/package_index.tar.bz2",
"file": "Downloading index: package_index.tar.bz2",
"totalSize": "43541"
}
}
{
"downloadProgress": {
"downloaded": "43541"
}
}
{
"downloadProgress": {
"completed": true
}
}
{
"downloadResult": {
"url": "https://downloads.arduino.cc/packages/package_index.tar.bz2",
"successful": true
}
}
{
"downloadProgress": {
"url": "http://example.com/package_foo_index.json",
"file": "Downloading index: package_foo_index.json",
"totalSize": "-1"
}
}
{
"downloadProgress": {
"downloaded": "1256"
}
}
{
"downloadResult": {
"url": "http://example.com/package_foo_index.json",
"error": "Error downloading index 'http://example.com/package_foo_index.json': Server responded with: 404 Not Found"
}
}
{
"downloadResult": {
"url": "http://example.com/package_foo_index.json",
"successful": true
}
}
ERROR:
Code: Internal
Message: Some indexes could not be updated.
🐛 The value of the cc.arduino.cli.commands.v1.ArduinoCoreService.DownloadResult.successful field was true for the failed download of http://example.com/package_foo_index.json.
Expected behavior
The value of the cc.arduino.cli.commands.v1.ArduinoCoreService.DownloadResult.successful field is set to false when the download fails.
🐛 The value of the cc.arduino.cli.commands.v1.ArduinoCoreService.DownloadResult.successful field was true for the failed download of http://example.com/package_foo_index.json.
I've pushed the fix for this problem, and the other one #1866 (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.
The bugs reported in my previous review have been resolved and it is working as expected now.
Thanks Cristian!
c82517d to
9fddd77
Compare
Please check if the PR fulfills these requirements
before creating one)
our contributing guidelines
UPGRADING.mdhas been updated with a migration guide (for breaking changes)core update-indexdo not stop anymore at the first errorUpdateIndexgRPC call has a new flagignore_custom_package_indexesto allow skipping the update of custom package indexes on the first run.If there is a problem downloading a package index the
core update-indexcommand will stop and not complete the update of the remaining custom indexes url.The
core update-indexcommand will always complete the update of all indexes.Yes
Fix [gRPC] It should be possible to run the
core update-indexequivalent with and without the 3rd party URLs #1788