Add a flag --skip-cached to skip already-downloaded models and collections#461
Add a flag --skip-cached to skip already-downloaded models and collections#461khaledgabr77 wants to merge 3 commits intogazebosim:gz-fuel-tools10from
Conversation
Signed-off-by: khaledgabr77 <khaledgabr77@gmail.com>
…rlds Signed-off-by: khaledgabr77 <khaledgabr77@gmail.com>
ahcorde
left a comment
There was a problem hiding this comment.
You have introduced many changes which are not related with this PR, do you mind to restore them
| 'defaults' => false, | ||
| 'console' => false | ||
| 'console' => false, | ||
| 'skip_cached' => 'false' |
There was a problem hiding this comment.
| 'skip_cached' => 'false' | |
| 'skip_cached' => false |
| options['console'] = true | ||
| end | ||
| opts.on('-s', '--skip-cached', 'Skip downloading resources already in cache') do | ||
| options['skip_cached'] = 'true' |
There was a problem hiding this comment.
| options['skip_cached'] = 'true' | |
| options['skip_cached'] = true |
| end | ||
| when 'download' | ||
| Importer.extern 'int downloadUrl(const char *, const char *, const char *, const char *, unsigned int)' | ||
| Importer.extern 'int downloadUrl(const char *, const char *, const char *, const char *, unsigned int, const char *)' |
There was a problem hiding this comment.
| Importer.extern 'int downloadUrl(const char *, const char *, const char *, const char *, unsigned int, const char *)' | |
| Importer.extern 'int downloadUrl(const char *, const char *, const char *, const char *, unsigned int, bool)' |
There was a problem hiding this comment.
Hi @ahcorde
I followed the changes you suggested and updated both gz.cc and gz.hh to accept a bool parameter. However, when I tried to compile and run the code, I encountered the following issue:
Library error: Problem running [download]() from /root/gz_ws/install/lib/libgz-fuel_tools10.so.10.0.1
I need to determine whether this issue came from the Ruby script or the C++ implementation. Previously, when reading the file, I noticed that char was used to represent boolean values, so I followed the same approach. However, now I plan to recheck the full code to understand exactly where the issue is coming from.
You're right — I forgot to mention that the changes are related to running |
ahcorde
left a comment
There was a problem hiding this comment.
You have introduced many changes which are not related with this PR, do you mind to restore them
You're right — I forgot to mention that the changes are related to running
ament_uncrustify --reformat. After I finished working on eachC++file, I applied the reformatting using that command. Do you still want me to revert the changes?
we are not using uncrustify in gazebo. Yes, restore the changes
I'm sorry for that, i restore changes now. |
0add1b3 to
053fe4c
Compare
Signed-off-by: khaledgabr77 <khaledgabr77@gmail.com>
053fe4c to
f3860b9
Compare
|
I’ve restored the changes and will review the updates you requested later tomorrow. |
ahcorde
left a comment
There was a problem hiding this comment.
some linters are failing
/github/workspace/src/gz.cc:468: Lines should be <= 80 characters long [whitespace/line_length] [2]
/github/workspace/src/gz.cc:515: If/else bodies with multiple statements require braces [readability/braces] [4]
/github/workspace/src/gz.cc:516: Lines should be <= 80 characters long [whitespace/line_length] [2]
/github/workspace/src/gz.cc:559: If/else bodies with multiple statements require braces [readability/braces] [4]
/github/workspace/src/gz.cc:560: Lines should be <= 80 characters long [whitespace/line_length] [2]
/github/workspace/src/gz.cc:658: Lines should be <= 80 characters long [whitespace/line_length] [2]
/github/workspace/src/gz.hh:65: Lines should be <= 80 characters long [whitespace/line_length] [2]
|
Hi @khaledgabr77, there are some review comments that are still not addressed. Do you think you'll have time to get to them before the Jetty code freeze (see https://discourse.openrobotics.org/t/feature-freeze-for-gazebo-jetty/48187/3?u=azeey) |
|
Thanks for the great contribution, but since there hasn't been any updates for a while, I'll remove it from the Jetty milestone. But please don't let this stop you from pushing on this PR. Once the initial feature freeze window is over, we'll be able to merge this. |
🎉 New feature
Closes #150
Summary
This PR introduces a
--skip-cachedflag option to gz-fuel-tools10 (available through thegz fuel downloadcommand).When the flag is set, any models that already exist in the local cache are skipped instead of downloaded again, saving time.
Both individual model downloads and collection downloads now respect this flag.
Behaviour before this PR
Every time running of
gz fuel downloadre-download of all models, even if they were already present locally.Behaviour after this PR
With
--skip-cached, the CLI first checks the local cache and only downloads the models that are missing.Test it
gz fuel download -j 4 -u "https://fuel.gazebosim.org/1.0/openrobotics/collections/Gazebo classic model database"gz fuel download -j 4 --skip-cached -u "https://fuel.gazebosim.org/1.0/openrobotics/collections/Gazebo classic model database"You can repeat the same steps with an individual model URL too.
Checklist
codecheckpassed (See contributing)Note to maintainers: Remember to use Squash-Merge and edit the commit message to match the pull request summary while retaining
Signed-off-bymessages.