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.

Sign up for GitHub

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

Jump to bottom

Extended support for filtering files copied in the exchange #1108

Merged
merged 6 commits into from
May 31, 2019
Merged

Extended support for filtering files copied in the exchange #1108

merged 6 commits into from
May 31, 2019

Conversation

nthiery
Copy link
Contributor

@nthiery nthiery commented May 30, 2019

  • ignore too large files
  • ignore non explicitly included files

Resolves #874.

@nthiery
Extended support for filtering files copied in the exchange
6ed181b 
- ignore too large files
- ignore non explicitly included files
@nthiery
Copy link
Contributor Author

nthiery commented May 30, 2019

Reviews welcome.
Not fully tested yet, in particular the warnings about ignored files.
Don't merge!

jhamrick
jhamrick reviewed May 31, 2019
View reviewed changes
Copy link
Member

@jhamrick jhamrick left a comment

Choose a reason for hiding this comment

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

Overall I think this looks great, thanks a lot! I don't have any major comments, but it would be great if you wanted to include some integration tests too 🙂

nbgrader/utils.py Show resolved Hide resolved
nbgrader/utils.py Outdated Show resolved Hide resolved
@jhamrick jhamrick added this to the 0.6.0 milestone May 31, 2019
nthiery added 3 commits May 31, 2019 11:06
@nthiery
Extended support for filtering files copied in the exchange: doc + tw…
a561848 
…eaks

- Only files too large trigger a warning. Otherwise just a debug message.
- Quote filenames in the log.
@nthiery
Typo
16b2632
@nthiery
Extended support for filtering files copied in the exchange: integrat…
61ec654 
…ion tests
@nthiery
Copy link
Contributor Author

nthiery commented May 31, 2019

Thanks Jessica for the review! All requested changes are done now.

jhamrick
jhamrick approved these changes May 31, 2019
View reviewed changes
Copy link
Member

@jhamrick jhamrick left a comment

Choose a reason for hiding this comment

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

Looks great, thanks a lot!

nbgrader/coursedir.py Outdated
help=dedent(
"""
List of file names or file globs: non matching files
will be ignored with a warning.
Copy link
Member

Choose a reason for hiding this comment

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

Maybe say you need to use the DEBUG log level to see the warning?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Ah, good point, forgot to update that part.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done, and polished a bit the option documentation at this occasion.

Copy link
Member

Choose a reason for hiding this comment

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

Thanks!

@jhamrick
Copy link
Member

Hmm, seems that the nbgrader/tests/utils/test_utils.py::test_ignore test is unhappy?

@nthiery
Copy link
Contributor Author

nthiery commented May 31, 2019

Oops, I indeed had forgotten to update the tests after a function name update. Fixed!

@jhamrick jhamrick merged commit 5a91bae into jupyter:master May 31, 2019
@nthiery nthiery deleted the filter-file-submissions branch June 3, 2019 22:11
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@jhamrick jhamrick jhamrick approved these changes

Assignees
No one assigned
Labels
enhancement
Projects
None yet
Milestone
0.6.0
Development

Successfully merging this pull request may close these issues.

Don't allow submitting of huge files
2 participants
@nthiery @jhamrick