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

ENH(UX): log a hint to use ulimit command in case of "Too long" exception #6173

merged 1 commit into from Nov 24, 2021


Copy link

Closes #6106

Fragile part of the test -- we might not hit that limit may be? Let's see what CI says. Alternative would be to mock that Popen call but that would get us away from reality.

Sure thing could be improved etc, but overall I am giving up on trying to set/control that CMD_MAX_ARG and decided just to hint (related: #6114) user on how to mediate.

Copy link

codecov bot commented Nov 10, 2021

Codecov Report

Merging #6173 (78dcad8) into maint (0df06be) will decrease coverage by 2.03%.
The diff coverage is 100.00%.

❗ Current head 78dcad8 differs from pull request most recent head 3dd89eb. Consider uploading reports for the commit 3dd89eb to get more accurate results
Impacted file tree graph

@@            Coverage Diff             @@
##            maint    #6173      +/-   ##
- Coverage   90.21%   88.18%   -2.04%     
  Files         312      312              
  Lines       42235    42230       -5     
- Hits        38104    37241     -863     
- Misses       4131     4989     +858     
Impacted Files Coverage Δ
datalad/runner/ 99.01% <100.00%> (-0.99%) ⬇️
datalad/runner/tests/ 93.24% <100.00%> (-1.80%) ⬇️
datalad/ 0.00% <0.00%> (-100.00%) ⬇️
datalad/plugin/ 0.00% <0.00%> (-100.00%) ⬇️
datalad/plugin/ 0.00% <0.00%> (-100.00%) ⬇️
datalad/plugin/ 0.00% <0.00%> (-100.00%) ⬇️
datalad/plugin/ 0.00% <0.00%> (-100.00%) ⬇️
datalad/plugin/ 0.00% <0.00%> (-100.00%) ⬇️
datalad/support/tests/ 0.00% <0.00%> (-100.00%) ⬇️
datalad/plugin/ 0.00% <0.00%> (-100.00%) ⬇️
... and 88 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 0df06be...3dd89eb. Read the comment docs.

@yarikoptic yarikoptic added semver-patch Increment the patch version when merged UX user experience labels Nov 10, 2021
Copy link

@mih mih left a comment

Choose a reason for hiding this comment

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

Wouldn't this be limited to non-localized machines?

$ ls /var/cache/apt/archives/* 
bash: /bin/ls: Die Argumentliste ist zu lang

Copy link
Member Author

we already rely on parsing error messages from git IIRC, and for that

datalad/runner/        git_env['LC_MESSAGES'] = 'C'

so I would assume that even in your case our GitRunner (which will use the base runner) would report in English I would assume. Would have been nice to check. Sure thing if runner is used directly, and without tune up of env -- no hint would be produced. But since it is just a hint -- better to have a hint in some (if not most) of the cases than none IMHO.

@mih mih merged commit abcc1ab into datalad:maint Nov 24, 2021
Copy link

mih commented Nov 24, 2021

Thanks for the explanation. Let's merge.

@yarikoptic yarikoptic deleted the bf-ulimit branch April 5, 2022 15:58
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
semver-patch Increment the patch version when merged UX user experience
None yet

Successfully merging this pull request may close these issues.

None yet

2 participants