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

Handle datapackage.json that may cause 500 errors #355

Closed
2 of 3 tasks
zelima opened this issue Apr 19, 2017 · 3 comments
Closed
2 of 3 tasks

Handle datapackage.json that may cause 500 errors #355

zelima opened this issue Apr 19, 2017 · 3 comments
Assignees

Comments

@zelima
Copy link
Contributor

zelima commented Apr 19, 2017

Sometimes we have cases, when datasets pass the Data Package validation and are published, but still throwing 500 errors.

Tasks

  • discuses the pros and cos for all available options (see below) or suggest new, better ones
  • decide which option better fits us (see solutions part)
  • Implement

Analysis

Reason for this is that even after validation there still may remain beyond Frictionless metadata specifications. Eg licenses property should be list of objects, not directly object etc and DP validation does not cover this kind of "mistakes" from users. Problem is that on server side we follow "rules" and always waiting that metadata (datapackage.json) will follow specifications.

We faced issue exactly like this with Geo data packages - you can see deeper discussions here #335

The problem is that, we can not rely on user and assume he/she will always set licenses to be a list of the objects, bot not either "overload" and ask to learn all the specifications.

We have several options:

  • have the flag inside jinja templates and check for type.
  • Modify on server side - have helper function that checks if it is list and appends to list if not
  • Modify the validation script, so that it throws warning/error if licenses is not set according to specs. This way we are guaranteed that it will always be list of the objects

Personally I think most reliable way to avoid this kind of error in the future is 3rd option (to modify validation script and worn users before publishing). But this is from perspective of developer.

We should consider that users do not want to sit and learn all the tiny details. They wan't to publish their data and that should be as simple as possible

Solution

As right now the 500 error might caused only in because Jinja template is expecting zeroth element from object Eg:

<div class="license">
        <i class="fa fa-gavel"></i> <a href="{{dataset.licenses[0].url}}" title="Available under the following License">{{dataset.licenses[0].name or dataset.licenses[0].id}}</a>
</div>

I propose to have a helper function in logic layer validate_for_template(descriptor) that will check type of licenses and just pop licenses from descriptor if not valid

@zelima
Copy link
Contributor Author

zelima commented Apr 28, 2017

@rufuspollock If it is ok to fix (hack) this on server side for now, let's do this in this milestone.

@rufuspollock
Copy link
Contributor

@zelima - agree with working on this issue. Re the analysis:

  • you want to think at the general level - not just this specific item of licenses but how to "normalize" and validate datapackage.json generally
    • do you understand what i mean by "normalize"
  • how and where would you do this in the system? e.g. on datapackage.json import to MetaStore, or attempt to render or ...
  • how would you handle a datapackage.json that even after normalization was invalid ...?

@zelima zelima self-assigned this May 8, 2017
zelima added a commit that referenced this issue May 10, 2017
zelima added a commit that referenced this issue May 10, 2017
- includes small helper function that validates descriptor - tested
- logic updated and test included to check invalid licenses 

refs #355
@zelima
Copy link
Contributor Author

zelima commented May 11, 2017

FIXED see solution section in analyses

@zelima zelima closed this as completed May 11, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants