Skip to content

jsk_data: download_data.py: ensure chmod downloaded data if possible#1571

Merged
k-okada merged 3 commits into
jsk-ros-pkg:masterfrom
furushchev:fix-jsk-data
Nov 14, 2017
Merged

jsk_data: download_data.py: ensure chmod downloaded data if possible#1571
k-okada merged 3 commits into
jsk-ros-pkg:masterfrom
furushchev:fix-jsk-data

Conversation

@furushchev
Copy link
Copy Markdown
Member

  • Fix: Make sure to change permission even if downloading is interrupted
  • Fix: Check file permission before chmod since os.chmod fails when changing the permission of files which were already changed.

@furushchev furushchev requested a review from wkentaro November 2, 2017 05:46
@furushchev furushchev self-assigned this Nov 2, 2017
@furushchev furushchev added this to the 2.2.6 milestone Nov 2, 2017
Comment thread jsk_data/src/jsk_data/download_data.py Outdated
if not is_file_writable(path):
os.chmod(path, 0777)
orig_path = osp.splitext(path)[0] + '.orig.bag'
if not is_file_writable(path):
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Maybe path -> orig_path?

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

@wkentaro nice catch!

@furushchev
Copy link
Copy Markdown
Member Author

@wkentaro Any other comments?

@furushchev
Copy link
Copy Markdown
Member Author

@wkentaro Thanks!
@k-okada Please merge if it's ok for you! 👍

@furushchev
Copy link
Copy Markdown
Member Author

@k-okada Please merge this if there is no problem, since without this pull request building jsk_recognition packages with shared downloaded data fails in some environments.

@furushchev
Copy link
Copy Markdown
Member Author

wait for #1573

@k-okada k-okada merged commit 1fac68f into jsk-ros-pkg:master Nov 14, 2017
@furushchev furushchev deleted the fix-jsk-data branch November 14, 2017 06:47
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.

3 participants