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

elements: urls.value should not be validated as URI #280

Closed
jacquerie opened this issue Nov 26, 2017 · 5 comments
Closed

elements: urls.value should not be validated as URI #280

jacquerie opened this issue Nov 26, 2017 · 5 comments
Assignees

Comments

@jacquerie
Copy link
Contributor

jacquerie commented Nov 26, 2017

We currently have

value:
description: |-
:MARC: ``8564_u``
format: uri
type: string
but this is too strict, since we use this field to store information like

<datafield tag="856" ind1="4" ind2=" ">
  <subfield code="u">@particleist</subfield>
  <subfield code="y">TWITTER</subfield>
</datafield>

which would become

{
    "urls": [
        {
            "description": "TWITTER",
            "value": "@particleist"
        }
    ]
}

but @particleist isn't a URI.

@jacquerie jacquerie changed the title elements: urls.value should not be validated as URIs elements: urls.value should not be validated as URI Nov 26, 2017
@jacquerie
Copy link
Contributor Author

Thing is, it makes a lot of sense to validate URLs as URIs, while it makes less sense to store there what are essentially external system identifiers. So my proposal is to relax temporarily this field to unblock inspirehep/inspire-next#2992 and inspirehep/inspire-next#2904 ASAP, and then restrict again when we have another place in the schema to put this information.

@michamos
Copy link
Contributor

Not sure I understand. The content of those fields are not URLs but external system identifiers, so they should be moved there. Note that this is already foreseen in commit e5a1ff4 (part of #182), that adds twitter and linkedin to the allowed author ids. In the meantime, those authors will fail migration. What's wrong with that?

@jacquerie
Copy link
Contributor Author

jacquerie commented Nov 26, 2017

In the meantime, those authors will fail migration. What's wrong with that?

One of the authors that fails migration is part of our demo set of records, which means we can't bump the schemas (which is happening in commit inspirehep/inspire-next@2afcbb0 of inspirehep/inspire-next#2992), which is blocking the manual merger from being merged.

@michamos
Copy link
Contributor

can't you patch the demo records then? It seems cleaner to me to do a temporary patch at the point of failure rather than here.

@jacquerie
Copy link
Contributor Author

can't you patch the demo records then? It seems cleaner to me to do a temporary patch at the point of failure rather than here.

👍

Will do like you suggest, and thanks for the weekend answer...

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

No branches or pull requests

2 participants