dperf icon indicating copy to clipboard operation
dperf copied to clipboard

return error message if the mount point is not writable

Open makeittotop opened this issue 2 years ago • 2 comments

Hi @harshavardhana Please let me know if this PR can be approved and merged?

Currently dperf fails silently if the directory at a given mount path is not writable. For example -

$ touch /tmp/foo1/.tmpFile
touch: cannot touch '/tmp/foo1/.tmpFile': Permission denied

$ dperf /tmp/foo1
┌───────────┬────────────┐
│ TotalREAD │ TotalWRITE │
│ 0 B/s     │ 0 B/s      │
└───────────┴────────────┘

$ echo $?
0

The cmd should instead return an error code and a meaningful error message to stderr as it does in case a directory at the mount path doesn't exists.

$ stat /tmp/foo
stat: cannot statx '/tmp/foo/': No such file or directory

$ dperf /tmp/foo         

$ echo $?
1

With the proposed changes -

$ dperf /tmp/foo1
ERROR directory at path '/tmp/foo1' is not writable

$ echo $?               
1

$ dperf /tmp/foo1 --verbose
ERROR directory at path '/tmp/foo1' is not writable

$ echo $?               
1

makeittotop avatar Oct 05 '23 21:10 makeittotop

Hi @harshavardhana Please let me know if this PR can be approved and merged?

Currently dperf fails silently if the directory at a given mount path is not writable. For example -

$ touch /tmp/foo1/.tmpFile
touch: cannot touch '/tmp/foo1/.tmpFile': Permission denied

$ dperf /tmp/foo1
┌───────────┬────────────┐
│ TotalREAD │ TotalWRITE │
│ 0 B/s     │ 0 B/s      │
└───────────┴────────────┘

$ echo $?
0

The cmd should instead return an error code and a meaningful error message to stderr as it does in case a directory at the mount path doesn't exists.

$ stat /tmp/foo
stat: cannot statx '/tmp/foo/': No such file or directory

$ dperf /tmp/foo         

$ echo $?
1

With the proposed changes -

$ dperf /tmp/foo1
ERROR directory at path '/tmp/foo1' is not writable

$ echo $?               
1

$ dperf /tmp/foo1 --verbose
ERROR directory at path '/tmp/foo1' is not writable

$ echo $?               
1

This is not actually correct, what we need to show is the error per drive in the table.

harshavardhana avatar Oct 07 '23 03:10 harshavardhana

Ah, I see what you mean. I can work on that.

On Fri, Oct 6, 2023 at 8:17 PM Harshavardhana @.***> wrote:

Hi @harshavardhana https://github.com/harshavardhana Please let me know if this PR can be approved and merged?

Currently dperf fails silently if the directory at a given mount path is not writable. For example -

$ touch /tmp/foo1/.tmpFile touch: cannot touch '/tmp/foo1/.tmpFile': Permission denied

$ dperf /tmp/foo1 ┌───────────┬────────────┐ │ TotalREAD │ TotalWRITE │ │ 0 B/s │ 0 B/s │ └───────────┴────────────┘

$ echo $? 0

The cmd should instead return an error code and a meaningful error message to stderr as it does in case a directory at the mount path doesn't exists.

$ stat /tmp/foo stat: cannot statx '/tmp/foo/': No such file or directory

$ dperf /tmp/foo

$ echo $? 1

With the proposed changes -

$ dperf /tmp/foo1 ERROR directory at path '/tmp/foo1' is not writable

$ echo $? 1

$ dperf /tmp/foo1 --verbose ERROR directory at path '/tmp/foo1' is not writable

$ echo $? 1

This is not actually correct, what we need to show is the error per drive in the table.

— Reply to this email directly, view it on GitHub https://github.com/minio/dperf/pull/9#issuecomment-1751581269, or unsubscribe https://github.com/notifications/unsubscribe-auth/ABFYC7K4ZGBHIWPMI3WPDT3X6DCS5AVCNFSM6AAAAAA5U3Q4ACVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMYTONJRGU4DCMRWHE . You are receiving this because you authored the thread.Message ID: @.***>

makeittotop avatar Oct 07 '23 03:10 makeittotop