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

BF: annexjson2result() did not report 'error-messages' (fixes gh-3870) #3931

Merged
merged 1 commit into from Dec 17, 2019

Conversation

mih
Copy link
Member

@mih mih commented Dec 16, 2019

Such information is now extracted from the annex report, and used
preferentially as 'message' when competing with a simultaneously
present 'note'. While this may hide some information, the omitted
parts are arguably less important and obfuscate any error. Without
this omission, a 'disk full' error may look like:

   get(error): sub-01/ses-movie/func/sub-01_ses-movie_task-movie_run-1_bold.nii.gz (file) [  not enough free space, need 31.04 MB more (use --force to override this check or adjust annex.diskreserve); Unable to access these remotes: mddatasrc; Try making some of these repositories available:;     2f51364e-815a-4ad9-b380-617c682b1680 -- [mddatasrc];    40809380-c618-4801-bb8d-81320474c9da -- studyforrest phase2 original source repository]

Also: #3930

…gh-3870)

Such information is now extracted from the annex report, and used
preferentially as 'message' when competing with a simultaneously
present 'note'. While this may hide some information, the omitted
parts are arguably less important and obfuscate any error. Without
this omission, a 'disk full' error may look like:

   get(error): sub-01/ses-movie/func/sub-01_ses-movie_task-movie_run-1_bold.nii.gz (file) [  not enough free space, need 31.04 MB more (use --force to override this check or adjust annex.diskreserve); Unable to access these remotes: mddatasrc; Try making some of these repositories available:;     2f51364e-815a-4ad9-b380-617c682b1680 -- [mddatasrc];    40809380-c618-4801-bb8d-81320474c9da -- studyforrest phase2 original source repository]
@mih
Copy link
Member Author

@mih mih commented Dec 16, 2019

Appveyor failed to submit coverage in 1/2 runs.

@yarikoptic
Copy link
Member

@yarikoptic yarikoptic commented Dec 16, 2019

oh, now it is in the 1st run, for me it was in the 2nd, so indeed it is just in 1/2 of runs, weird

@mih
Copy link
Member Author

@mih mih commented Dec 16, 2019

It tends to be random.

messages.extend(d['error-messages'])
# avoid meaningless standard messages, and collision with actual error
# messages
elif 'note' in d:
Copy link
Member

@yarikoptic yarikoptic Dec 16, 2019

Choose a reason for hiding this comment

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

I do agree that errors are more important, but I wonder if note should also be retained?
i.e. elif to be changed back to if?

Copy link
Member Author

@mih mih Dec 16, 2019

Choose a reason for hiding this comment

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

That is the example I posted. I think it makes it worse.

Copy link
Member

@yarikoptic yarikoptic Dec 16, 2019

Choose a reason for hiding this comment

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

and there is no way to keep error message "separate" so it could e.g. get a distinctive color (red) while rendering the result?

Copy link
Member Author

@mih mih Dec 17, 2019

Choose a reason for hiding this comment

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

We don't have that yet, but I recorded the need #3939

@mih
Copy link
Member Author

@mih mih commented Dec 17, 2019

Minus a future enhancement this seems to be ready, and I will merge it to get the issue resolved for now.

@mih mih merged commit b2fd4f3 into datalad:master Dec 17, 2019
14 of 15 checks passed
@mih mih deleted the bf-3870 branch Dec 17, 2019
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.

None yet

2 participants