-
Notifications
You must be signed in to change notification settings - Fork 22
feat: Ignore files from transfer #504
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
base: main
Are you sure you want to change the base?
feat: Ignore files from transfer #504
Conversation
Since it's not suggested to use--include and --exclude together, an alternative is to use a file (such as .datashuttleignore). In order to do it, some changes were required, and this commit addresses them.
…roved clarity and functionality
Hello @laredoo, If you are aiming for the GSoC'25, I suggest you, start drafting your proposal. The last date for applying is April 8th in the GSoC portal but your proposal needs to be reviewed in the gsoc repo before submission through PR. Currently the maintainer of the datashuttle project @JoeZiminski is on leave, he may take time to review this PR. If you want any suggestions or any questions regarding proposal don't hesitate to tag me. I'm happy to help you. Thank you |
Hey @sumana-2705, appreciate the heads-up! I already have a partial draft of my proposal for the DataShuttle project but haven't opened the PR yet. I'll create and submit it ASAP so there's enough time for review before the April 8th deadline, especially considering @JoeZiminski is currently on leave. I appreciate your offer to help with suggestions or questions regarding the proposal. I'll definitely tag you if I need assistance during the process. Thanks again for your support! |
Hey everyone! I’ve been running into some trouble replicating the CI environment to debug why this job and this one failed. I’ve already rebased my branch to sync with the latest changes, but since the tests run on a cron schedule, they haven’t triggered yet. |
Hey @laredoo. The tests randomly fail sometimes and this might be the case here. With this specific test, I did face issues while working on this PR. The code in your PR doesn't touch the code involved in the particular test that's failing so it's possible that it randomly failed. However, I noticed something very wierd. This particular test (both on your branch and on the main branch) always fails when my laptop (lenovo, running debian) is running on a battery saver mode and passes otherwise. It is very confusing to me as to why this happens. Maybe something random like this might have caused the tests to fail. |
Thanks for the feedback @cs7-shrey! Good to know I’m not the only one seeing this haha. Honestly, I'm also having trouble to understand why this is happening. The test always passes locally on my machine, which makes it even more confusing. Replicating the runner environment is a bit tricky, and I haven’t yet had the chance to properly do it. Hopefully, it’ll pass on the next run—but if not, I’ll try to dig deeper into it. Been focusing more on the application side lately, so haven’t had the time to properly debug it. |
Hi @laredoo thanks a lot for this PR! Sorry to see about the failing tests, thanks everyone for working together to discuss these issues. Indeed the tests can sporadically fail in the very difficult-to-detect ways. This is mostly due to testing the textual side of things, in some cases strange things can happen with the test application size being too small, clicks not registering etc. In the case of this PR please feel free to ignore these, I will re-run tests until they pass. I ran on windows and they passed so am no concerned these changes are effecting anything. Tracking down these random test failures can be the subject of a future PR, and may require out-of-scope changes (e.g. upgrading textual version). This is a very clean implementation, I played around with it locally and it looks great. I won't have time to review this in full prior to the 8th but please do include this as a GSoC contribution. This feature is one that will probably be included pending feedback from users. There is something of a balance between including features that are likely to be useful vs. adding too many features that will not be widely used. For now, we will probably hold off merging this and await more feedback from users on features that will be useful for them. Nonetheless this is an extremely useful contribution as a) it allows us to see what the idea expressed in issue #350 looks like in practice b) this can be quickly incorporated into the codebase if the feature is included. So thanks a lot for this valuable contribution! |
Hey @JoeZiminski , thanks a lot for the words and for taking the time to test things out locally — I really appreciate it! Glad to hear the tests are nothing to worry about on this PR. Made sure to also include this contribution in my GSoC application. Totally agree on the importance of balancing feature usefulness. I tried to implement the feature as flexible as possible (e.g. expanding it to other transfers options than custom) so it could be smoothly addapted based on user feedback! Overall, I’m really happy to see this being considered, and I’m more than willing to iterate on it if it moves forward later. Also, I’d be glad to help with that potential future PR — I somehow enjoy digging into those weird bugs and figuring out what’s going on haha. Thanks again for the support and feedback! |
Description
This pull request implements a new feature that allows the user to ignore files when transfering. It works similarly to a .gitignore file, so any regex (e.g *.mp4) should be accepted.
Several tests have been done locally and the feature has performed quite well.
Another key point is that currently only custom transfers support excluding files. It shouldn't an effortful task to expand it to all other transfers options, since it has been implemented to be flexible (heavy testing have been done with all transfer options).
What is this PR
Why is this PR needed?
This PR is needed to provide users with the ability to exclude specific files during transfers using regex patterns, similar to a .gitignore file. This enhances flexibility and control over file transfers, reducing unnecessary data movement. While currently limited to custom transfers, the implementation is designed to be easily extendable to other transfer options.
What does this PR do?
This PR introduces a feature that allows users to ignore files during transfers using regex patterns, similar to a .gitignore file. It currently applies to custom transfers but is designed to be easily extendable to other transfer options.
References
Add an 'ignore' (e.g. like .gitignore feature to selectively ignore certain filetypes / folders from transfer)
How has this PR been tested?
This PR has been tested through extensive local testing across all transfer options to ensure flexibility and reliability. However, additional testing is still pending, including unit tests and validation on a Windows machine.
Is this a breaking change?
No, this is not a breaking change. The feature is implemented in a backward-compatible manner, ensuring that existing transfer functionality remains unaffected.
If this PR breaks any existing functionality, please explain how and why.
So far, this PR has not broken any existing functionality. The implementation maintains backward compatibility, and extensive local testing has confirmed that all transfer options continue to function as expected.
Does this PR require an update to the documentation?
The documentation has not yet been updated since this PR is still in draft. Once finalized, the documentation will need to include details on the new file exclusion feature, explaining how users can specify patterns to ignore files during transfers, its current limitation to custom transfers, and any future plans for expanding support.
Checklist: