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

Added support for warning about and skipping files. #881

Merged
merged 3 commits into from
Aug 19, 2014

Conversation

kyleknap
Copy link
Contributor

Files that do not exist or user does not have read access to
are skipped and users are warned that the file is skipped.

@kyleknap
Copy link
Contributor Author

Fixes: #856

cc @jamesls @danielgtaylor

@coveralls
Copy link

Coverage Status

Coverage decreased (-0.03%) when pulling 4932e30 on kyleknap:warnings into 46d5acd on aws:develop.

@@ -625,8 +630,10 @@ def run(self):
# keeping it simple and saying that > 0 failed tasks
# will give a 1 RC.
rc = 0
if files[0] > 0:
Copy link
Member

Choose a reason for hiding this comment

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

The comment above this is out of date. We should get this comment updated.

@jamesls
Copy link
Member

jamesls commented Aug 14, 2014

Looks good, just a couple of minor points:

  • Looks like coverage dropped here. Let's try to keep each pull request maintaining or increasing coverage.
  • In addition to access permissions, I think it would also be worth checking special file types on posix systems (fifos, domain sockets, etc.) by adding some type of is_special_file function in compat.py.

@coveralls
Copy link

Coverage Status

Coverage increased (+0.06%) when pulling c414694 on kyleknap:warnings into 09bbb5e on aws:develop.

@danielgtaylor
Copy link
Contributor

LGTM 👍

@kyleknap
Copy link
Contributor Author

Newest version is compatible with Windows. This includes being able to ignore files that do not exists and files that the user does not have permission to.

Also, pull request now fixes:
Fixes: #782

if os.path.isdir(path):
try:
os.listdir(path)
except Exception:
Copy link
Member

Choose a reason for hiding this comment

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

I would expected these to only catch their appropriate errors OSError (and IOError in py2).

@jamesls
Copy link
Member

jamesls commented Aug 19, 2014

:shipit: Looks good! Also, a few things to watch out for:

  • Top level classes/functions should have two blank lines between them. There were a few test classes with only a single line.
  • For docstrings, try to stick to pep 257, with the exception that we use the sphinx style for documenting args, i.e :param foo: The foo of bar.

Don't forget to update the changelog as well.

@coveralls
Copy link

Coverage Status

Coverage increased (+0.09%) when pulling 0166cd5 on kyleknap:warnings into a824a8e on aws:develop.

Files that do not exist or user does not have read access to
are skipped and users are warned that the file is skipped.
Files include character special devices, block special devices,
FIFO, and sockets.
@coveralls
Copy link

Coverage Status

Coverage increased (+0.09%) when pulling 8b1320e on kyleknap:warnings into a824a8e on aws:develop.

kyleknap added a commit that referenced this pull request Aug 19, 2014
Added support for warning about and skipping files.
@kyleknap kyleknap merged commit 9d311fa into aws:develop Aug 19, 2014
@kyleknap kyleknap deleted the warnings branch August 19, 2014 18:12
thoward-godaddy pushed a commit to thoward-godaddy/aws-cli that referenced this pull request Feb 12, 2022
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.

4 participants