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

[BUG] Component header and Freetext Value cause exception #19

Open
omaus opened this issue Jan 21, 2025 · 19 comments · Fixed by #20
Open

[BUG] Component header and Freetext Value cause exception #19

omaus opened this issue Jan 21, 2025 · 19 comments · Fixed by #20

Comments

@omaus
Copy link
Collaborator

omaus commented Jan 21, 2025

Describe the bug
ValidateAgainstHeader causes an error in everyday usecase.

To Reproduce
Steps to reproduce the behavior:

  1. Create new ARC
  2. Fill out investigation metadata
  3. Create study and fill out metadata
  4. Test pipelines, should pass
  5. Add new metadata sheet to study and add Component building block
  6. Test pipeline, should succeed
  7. Add a row to your Component building block
  8. Test pipeline, pipeline fails with metadata error
  9. Change component to something else, e.g. Characteristics
  10. Pipeline succeeds

Expected behavior
Everyting's fine.

OS and framework information (please complete the following information):

  • OS: Win10
  • OS Version: Pro 64-bit (10.0, Build 19045)

Additional context
Full history of the bug occurence.

@HLWeil
Copy link
Member

HLWeil commented Jan 22, 2025

So Pipeline does not flatout fail Image

Just added a study with a component column with two values.

@HLWeil
Copy link
Member

HLWeil commented Jan 22, 2025

Or is this the Issue:

Image

@SabrinaZander
Copy link

SabrinaZander commented Jan 22, 2025

The overall pipeline seems to pass, but when you have a look at the details it looks like that.

Image

The Publication Metadata fails and then you cannot publish the ARC.

Image

@HLWeil
Copy link
Member

HLWeil commented Jan 23, 2025

Ahh so the Invenio package needs to be referenced?

@SabrinaZander
Copy link

Yes, sorry, that's missing in the steps to reproduce.

Maybe just for the sake of completeness. We had an ARC that we wanted to publish. The Invenio validation package had to be added for this. Then this metadata error occurred in the pipeline. The ARC could not be published because the ARChigator page always showed the error "Could not load metadata".
According to the error description in the pipeline, it has something to do with the Swate building blocks. After a lot of testing, I found out that it was the component building block. I can't say exactly where the error came from. But after I replaced all the component building blocks, the error disappeared and the ARC could be published. See here again:

nfdi4plants/arc-validate#112

I can also invite you to some test ARCs if you want to.

@HLWeil
Copy link
Member

HLWeil commented Jan 23, 2025

Okay I see. I could reproduce the error now after referencing the Invenio Test package and looking into it 👍

@HLWeil
Copy link
Member

HLWeil commented Jan 23, 2025

But another requirement for this to fail is, that the value in the cell needs to be freetext. Terms work just fine.

@HLWeil HLWeil changed the title [BUG] Component header and (seemingly) any value in cell cause exception [BUG] Component header and Freetext Value cause exception Jan 23, 2025
@HLWeil
Copy link
Member

HLWeil commented Jan 23, 2025

Okay this is already fixed in ARCtrl, should suffice to update the dependency in arc-to-invenio

@HLWeil HLWeil transferred this issue from nfdi4plants/ARCtrl Jan 23, 2025
@HLWeil HLWeil mentioned this issue Jan 23, 2025
@github-project-automation github-project-automation bot moved this from Backlog to Done in ARCStack Jan 23, 2025
@HLWeil
Copy link
Member

HLWeil commented Jan 23, 2025

Okay so the Issue should fixed.

@SabrinaZander @Hannah-Doerpholz would be great if you could confirm

@SabrinaZander
Copy link

Sorry, but now the error looks like this. Or do I need to do something else besides restarting the pipeline?

Image

@HLWeil HLWeil reopened this Jan 24, 2025
@github-actions github-actions bot added the Status: Needs Triage This item is up for investigation. label Jan 24, 2025
@HLWeil
Copy link
Member

HLWeil commented Jan 24, 2025

Maybe you could invite me to the ARC @SabrinaZander, so I can have a look?

@Hannah-Doerpholz
Copy link

Since I have the same issue in my test ARC I invited you to that one if that helps. There is no "secret" data in that one @HLWeil

@HLWeil
Copy link
Member

HLWeil commented Jan 28, 2025

Okay after some more investigation, it seems like this is not a problem of any of the source code.

Instead, the docker packaging of the tool does not work as intended, i.e. calling the tool fails.

Which also becomes apparent in the image you posted:

Image

@HLWeil
Copy link
Member

HLWeil commented Jan 28, 2025

Should be fixed with 0f4fba8

@Hannah-Doerpholz, I restarted your pipeline and it ran through. @SabrinaZander, could you also check?

@Hannah-Doerpholz
Copy link

It ran through for my actual "data" ARC as well. Thank you!

@SabrinaZander
Copy link

SabrinaZander commented Jan 29, 2025

Thank you very much, in my test ARC the pipeline runs successfully.
In another ARC, in which I first encountered the metadata pipeline problem, there is still an error.
Now called internal error:

Image

If you would like to investigate this further, I would be happy to invite you. It is no longer relevant for me, as we have already remade the ARC and published it.

@HLWeil
Copy link
Member

HLWeil commented Jan 29, 2025

Yeah, I'll take a look!

@HLWeil HLWeil removed the Status: Needs Triage This item is up for investigation. label Jan 29, 2025
@HLWeil
Copy link
Member

HLWeil commented Jan 30, 2025

Okay, this problem is actually a mismatch between the strictness of the validation and the conversion, regarding the information in the contacts:

  1. Validation passes if at least one person has all required fields*
  2. Converter requires all persons to have the required fields

In this case, one person was missing the affiliation property.

So, in principle the goal will be to align both tools to follow the same rules. I don't want to decide on which of the two approaches to take by myself, so I'll move this to a discussion.


*Required fields

  • First Name
  • Last Name
  • E-Mail
  • Affiliation

@HLWeil
Copy link
Member

HLWeil commented Jan 30, 2025

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Status: Done
Development

Successfully merging a pull request may close this issue.

4 participants