Use frictionless framework v5 #69

aivuk · 2022-09-22T09:25:07Z

Update the extension to use Frictionless Framework version 5. Change the validate function from Goodtables to the Frictionless equivalent.

TODO:

Update the Report component to display the validation results using the new React component.
Add more documentation in how to pass the option parameters to the validate function. With goodtables you could pass skip_rows and headers options that are not supported by the current validate.
Add tests for the new validate options

…ess framework v5

…s.py

…g it

…eValidationOnUpdateForm.test_resource_form_update_invalid

…herwise absolute paths are not allowed

…nless Framework documentation

amercader · 2022-09-22T12:10:09Z

@aivuk this PR should be against dev-v2 right?

aivuk · 2022-09-22T12:11:46Z

@aivuk this PR should be against dev-v2 right?

yes, sorry about that. I already changed it to dev-v2 now

ckanext/validation/jobs.py

codecov · 2022-09-23T15:45:35Z

Codecov Report

Merging #69 (fff8dc5) into dev-v2 (455090b) will decrease coverage by 0.16%.
The diff coverage is 88.23%.

@@            Coverage Diff             @@
##           dev-v2      #69      +/-   ##
==========================================
- Coverage   84.48%   84.31%   -0.17%     
==========================================
  Files          24       24              
  Lines        2062     2078      +16     
==========================================
+ Hits         1742     1752      +10     
- Misses        320      326       +6

Impacted Files	Coverage Δ
ckanext/validation/plugin/__init__.py	`96.24% <ø> (ø)`
ckanext/validation/tests/helpers.py	`100.00% <ø> (ø)`
ckanext/validation/jobs.py	`82.85% <81.25%> (-0.87%)`	⬇️
ckanext/validation/helpers.py	`88.88% <87.50%> (-0.86%)`	⬇️
ckanext/validation/logic.py	`68.27% <100.00%> (-0.13%)`	⬇️
ckanext/validation/tests/test_form.py	`100.00% <100.00%> (ø)`
ckanext/validation/tests/test_jobs.py	`100.00% <100.00%> (ø)`
ckanext/validation/tests/test_logic.py	`100.00% <100.00%> (ø)`

📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more

roll · 2022-10-25T07:31:14Z

TBH I don't fully understand a new approach. If there is still a field for CKAN end-users called "validation_options" - how they will be filling it if framework@5 doesn't even have this concept of validation options? I thought it was for internal representation, not user-facing.

aivuk · 2022-10-25T08:43:14Z

TBH I don't fully understand a new approach. If there is still a field for CKAN end-users called "validation_options" - how they will be filling it if framework@5 doesn't even have this concept of validation options? I thought it was for internal representation, not user-facing.

The ckanext-validation is running the function validate from frictionless framework. If an user wants to pass parameters to the validate as skip_errors or pick_errors, or customize the dialect, or pass parameters to the validation checks, s/he is going to do it passing a json in the "validation options" field.

Can you please elaborate on the first question (how to pass)?

I mean a simple list with all the checks options names as 'table-dimensions' and their parameters. The only list that I could find was for the baseline checks but there is no documentation for what each one does https://framework.frictionlessdata.io/docs/checks/baseline.html

aivuk · 2022-10-25T08:46:46Z

@roll Just an example, currently the user can put in the validation options field the json:

{
  "checks: [
    {"type": "table-dimensions", "minRows": 3},
    {"type": 'ascii-value'}
  ],
  "dialect": {
     "header": true,
     "headerRows": [1],
     "commentChar": "#",
     "csv": {
       "delimiter": ";"
     }
  }
}

amercader

@aivuk this looks good, just some polishing of the docs and tests and we can merge this

We can iron out things in separate PRs

README.md

ckanext/validation/jobs.py

amercader · 2022-10-25T11:13:14Z

ckanext/validation/templates/scheming/form_snippets/resource_schema.html

@@ -1,7 +1,6 @@
 {% import 'macros/form.html' as form %}

  {% set value = data[field.field_name] %}
-  {% set is_url = value and value[4:]|lower == 'http' %}


ping, I think this line should stay

amercader · 2022-10-25T11:14:03Z

ckanext/validation/tests/test_jobs.py

-
-        assert validation.report["valid"] is True
+# TODO: The vollowing test is not valid on frictionless v5. 
+# The local path is not hidden and there is no warning for more


I think these tests should be reactivated no?

amercader · 2022-10-25T11:16:14Z

requirements.txt

@@ -1,3 +1,5 @@
 ckantoolkit>=0.0.3
-goodtables==1.5.1
+frictionless==5.0.0b9


Sure, as soon as frictionless v5 goes out of beta we'll target the stable version

README.md

Co-authored-by: Adrià Mercader <amercadero@gmail.com>

amercader · 2022-10-25T11:35:16Z

@roll

TBH I don't fully understand a new approach. If there is still a field for CKAN end-users called "validation_options" - how they will be filling it if framework@5 doesn't even have this concept of validation options? I thought it was for internal representation, not user-facing.

We just need a way for users for inputing parameters for the validate() call. Right now we have this existing field which accepts a JSON object so it's easy to dump everything there rather than create separate checks, dialect, skip_errors etc fields in the form. The end goal should be to have a proper UI for tweaking all these settings, which I assume would be another frictionless component. When that's ready we'll refactor the form to use this and internally we'll settle on a way to pass the parameters to frictionless' validate() function

Co-authored-by: Adrià Mercader <amercadero@gmail.com>

…ssdata/ckanext-validation into use-frictionless-framework-v5

Co-authored-by: Adrià Mercader <amercadero@gmail.com>

…ssdata/ckanext-validation into use-frictionless-framework-v5

roll · 2022-10-25T14:03:00Z

@amercader
That totally sounds good as an intermediate step. Just wanted to ensure that we're on the same page regarding the final vision (a UI component field instead of the validation_options field)

ThrawnCA · 2022-10-26T01:21:21Z

.github/workflows/test.yml

@@ -49,6 +49,8 @@ jobs:
      run: |
        pip install -r dev-requirements.txt
        pip install -r requirements.txt
+        pip install --no-warn-conflicts jinja2==2.10.1


This flag is concerning. Why can't conflicts be resolved? There should at least be a comment to explain why this is being done and what would be needed to address it.

@ThrawnCA this will be solved with upcoming requirement changes in frictionless-py. We just added this to not block the tests on this branch.

@aivuk would be good to sort this out on frictionless-py. IIRC it's just loosening up the requirements

README.md

…wing frictionless v5

…port-component

…onlessdata/ckanext-validation into use-frictionless-report-component

…component Uses new frictionless report component to display validation report

amercader · 2022-10-27T09:31:48Z

Great work @aivuk ! 🚀

aivuk added 21 commits September 14, 2022 10:16

Update validation library to Frictionless version 5

bda364a

remove http proxy from validate because it doesn't exist in frictionl…

a8d8e09

…ess framework v5

put frictionless dependency to the last beta version 5.0.0b9

5021f95

Install jinja2 version 2.10.1 that works with CKAN 2.9

259016b

pin down markupsafe version

6e5a68a

add tableschema dependency

d57a3cf

Fix logic for new report format from frictionless v5

358bd0d

Fix some tests in test_logic

7c63f1a

Revert back test.ini

57030c6

Comment out test for goodtables

ac7bd2b

Comment out another test for goodtables

972d094

Fix tests in test_logic and check for different report formats at job…

cfa307d

…s.py

Merge branch 'dev-v2' into use-frictionless-framework-v5

92af068

correct test.ini file

e602d62

Do not use .get in frictionless Report object

cc3ff3d

Check first for key 'warnings' and 'tables' in Report before accessin…

5bbe208

…g it

Remove unnecessary json parsing and fix test at test_form TestResourc…

f75f84f

…eValidationOnUpdateForm.test_resource_form_update_invalid

Change how key check is done in Report object

d7df909

Uses frictionless.system to run validate in a trusted environment, ot…

cfd4305

…herwise absolute paths are not allowed

Correctly check resource source in Report and correct test in test_form

1a027f9

Update README removing references to Goodtables and adding to Frictio…

876ff6b

…nless Framework documentation

aivuk requested a review from amercader September 22, 2022 09:25

aivuk changed the base branch from master to dev-v2 September 22, 2022 12:11

aivuk self-assigned this Sep 22, 2022

ThrawnCA reviewed Sep 22, 2022

View reviewed changes

ckanext/validation/jobs.py Outdated Show resolved Hide resolved

Use http proxy with frictionless v5

28608e9

ThrawnCA approved these changes Sep 23, 2022

View reviewed changes

amercader reviewed Oct 25, 2022

View reviewed changes

Update README.md

a067c5c

Co-authored-by: Adrià Mercader <amercadero@gmail.com>

aivuk and others added 8 commits October 25, 2022 13:36

Update README.md

de8793a

Co-authored-by: Adrià Mercader <amercadero@gmail.com>

Update link to the current custom check of frictionless v5

372f40e

Merge branch 'use-frictionless-framework-v5' of github.com:frictionle…

2cb684f

…ssdata/ckanext-validation into use-frictionless-framework-v5

Update link to validate documentation from frictionless framework v5

bf2a50f

Update ckanext/validation/jobs.py

7a58f7c

Co-authored-by: Adrià Mercader <amercadero@gmail.com>

Add checks examples

17c33ee

Merge branch 'dev-v2' into use-frictionless-framework-v5

ca59320

Merge branch 'use-frictionless-framework-v5' of github.com:frictionle…

4c63d6e

…ssdata/ckanext-validation into use-frictionless-framework-v5

ThrawnCA reviewed Oct 26, 2022

View reviewed changes

ThrawnCA requested changes Oct 26, 2022

View reviewed changes

README.md Outdated Show resolved Hide resolved

Fix report dialog when plugin is running with sync=True

26f0203

amercader mentioned this pull request Oct 26, 2022

Uses new frictionless report component to display validation report #71

Merged

aivuk and others added 8 commits October 26, 2022 16:16

Fix tests for local paths in report and with validation options follo…

e9575a1

…wing frictionless v5

add missing closing quote

cf90c6b

Put is_url variable back on template

e774828

Make dialog of report dialog bigger and correct dialog css

b33dd11

Merge branch 'use-frictionless-framework-v5' into use-frictionless-re…

08af965

…port-component

remove console for debug in js

3da4388

Merge branch 'use-frictionless-report-component' of github.com:fricti…

c3b6807

…onlessdata/ckanext-validation into use-frictionless-report-component

Merge pull request #71 from frictionlessdata/use-frictionless-report-…

fff8dc5

…component Uses new frictionless report component to display validation report

amercader merged commit 88b52db into dev-v2 Oct 27, 2022

amercader deleted the use-frictionless-framework-v5 branch October 27, 2022 09:31

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Use frictionless framework v5 #69

Use frictionless framework v5 #69

aivuk commented Sep 22, 2022

amercader commented Sep 22, 2022

aivuk commented Sep 22, 2022

codecov bot commented Sep 23, 2022 •

edited

Loading

roll commented Oct 25, 2022

aivuk commented Oct 25, 2022

aivuk commented Oct 25, 2022 •

edited

Loading

amercader left a comment

amercader Oct 25, 2022

amercader Oct 25, 2022

amercader Oct 25, 2022

amercader commented Oct 25, 2022

roll commented Oct 25, 2022

ThrawnCA Oct 26, 2022 •

edited

Loading

amercader Oct 26, 2022

amercader commented Oct 27, 2022

Use frictionless framework v5 #69

Use frictionless framework v5 #69

Conversation

aivuk commented Sep 22, 2022

amercader commented Sep 22, 2022

aivuk commented Sep 22, 2022

codecov bot commented Sep 23, 2022 • edited Loading

Codecov Report

roll commented Oct 25, 2022

aivuk commented Oct 25, 2022

aivuk commented Oct 25, 2022 • edited Loading

amercader left a comment

Choose a reason for hiding this comment

amercader Oct 25, 2022

Choose a reason for hiding this comment

amercader Oct 25, 2022

Choose a reason for hiding this comment

amercader Oct 25, 2022

Choose a reason for hiding this comment

amercader commented Oct 25, 2022

roll commented Oct 25, 2022

ThrawnCA Oct 26, 2022 • edited Loading

Choose a reason for hiding this comment

amercader Oct 26, 2022

Choose a reason for hiding this comment

amercader commented Oct 27, 2022

codecov bot commented Sep 23, 2022 •

edited

Loading

aivuk commented Oct 25, 2022 •

edited

Loading

ThrawnCA Oct 26, 2022 •

edited

Loading