Feature/py3 #53

nickumia-reisys · 2021-07-31T00:39:57Z

I'm creating this PR a little early so that we have a somewhat good way to track changes from main. There were a few files that I broke up completely, for example, ckanext/usmetadata/plugin.py because it needed to be updated from controllers to blueprints and, since that's a big change, I stripped out constants into a separate file. Those constants are pretty important, but because I did that, we can't track the changes to those constants too well from this.

Either way, this is a huge upgrade. ... more comments to follow..

…erator' linting errors, so we are going to just ignore them; I thought these were originally strings which is why I attempted to join them, but they are some form of lists

…plugin parameters

…for this extension; started to fix py3 errors, but a lot of them stem from controller to blueprint issue, so trying to make fresh slate for that commit

…f things no longer existing and/or being rewritten, will try to see how the tests do..

…e to 'fanstatic' directory; fixed template reference to dashboard home (following example in 'ckanext-datagovtheme' ./ckanext/datagovtheme/templates/templates_2_8/header.html:46)

…en though I'm not using it directly; the 'before_search' function in the plugin is causing an issue because there is no good way to verify a user is authorized right now

…e controller to blueprint conversion

…ally; fixed lint; these changes will change a lot of the plugin functionality...

… taken which may cause incomplete data uploads; but until we know the proper way to update the validation, this was deemed the best approach

…visit soon

…s' is imported from future for py3 support

…est_validate_dataset_action and added additional validation tests as referenced in TODO

…he rest of the tests;... well... lint passes and the tests seem to work on CKAN 2.8.... moving on to CKAN 2.9 now? :s"

…works in 2.8 or is commented out because of a schema change in the plugin; fixed lint

…rror

nickumia-reisys · 2021-08-03T06:30:26Z

This PR is to complete this issue.

nickumia-reisys · 2021-08-03T07:01:39Z

@mogul Please remove the circleci checks 😅

mogul · 2021-08-03T15:34:51Z

@mogul Please remove the circleci checks 😅

Done.

…ake sense

…feature/py3

…t for the field; fixed test errors

nickumia-reisys · 2021-08-05T19:50:53Z

Bug Found via dcat_usmetadata testing:

_ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _

regex_candidate = <ckan.lib.navl.dictization_functions.Missing object at 0x7f980c22a8d0>

    def bureau_code_validator(regex_candidate):
        validator = re.compile(r'^\d{3}:\d{2}(\s*,\s*\d{3}:\d{2}\s*)*$')
>       if isinstance(validator.match(regex_candidate), type(re.match("", ""))):
E       TypeError: expected string or buffer

src/ckanext-usmetadata/ckanext/usmetadata/plugin/helper.py:26: TypeError

Will fix now..

jbrown-xentity

Most of these comments are minor tweaks/changes, great work!

jbrown-xentity · 2021-08-03T17:23:44Z

ckanext/usmetadata/tests/unit_test.py

+
+        converted_data, errors = df.validate(data, schema)
+
+        # TODO: schema approach was changed, so we don't care if it's missing right now


Can we add what the desired functionality is?

jbrown-xentity · 2021-08-06T13:59:24Z

ADR.md

+
+### Consequences
+
+- Until the schema is updated properly, incomplete input data may cause unknown bugs throughout the system.


This is a little vague and harsh. I would say "until the schema is properly updated, users may be able to input data that is not exportable in the data.json file. Due to the limited number of users and very little new users, this risk was deemed acceptable."

jbrown-xentity · 2021-08-06T14:00:17Z

ADR.md

+Custom vaildator functions were written to handle the functionality that the imported validators provided.  The following link was used as a reference,
+- https://docs.ckan.org/en/2.9/extensions/adding-custom-fields.html#custom-validators
+The validators were not registered with the plugin because the place where they were defined was already importing the plugin, so there would have
+been a circular dependency.  Either way, the validators work as standalong functions that get called.


Good catch 😄

jbrown-xentity · 2021-08-06T14:00:45Z

ADR.md

+
+### Consequences
+
+- The string_length_validator has known issues where sometimes it allows strings that are longer than allowed.  However, as a unit test, the function works.


We fixed this, correct?

jbrown-xentity · 2021-08-06T14:32:13Z

Dockerfile

@@ -0,0 +1,17 @@
+ARG CKAN_VERSION=2.8


Do we want to try to remove this whole file?

I'll add this to the ADR.

jbrown-xentity · 2021-08-06T17:46:29Z

requirements.txt

@@ -1 +1,7 @@
+flake8


Shouldn't these be in a requirements-dev.txt?

I'll add the dev-requirements.txt file

…out tests

jbrown-xentity

Thanks for the changes, looks great!

nickumia-reisys added 30 commits July 28, 2021 16:02

moved original tests into nose, updated local test suite, fixed py3 lint

df20b52

added github actions file; probably doesn't work yet

427e240

updated documentation for local testing; fixed circle build reference

bd415c3

recovered legacying linting for circleci build

8d7682a

I've never found a good way of avoiding 'break before/after binary op…

9dd66ca

…erator' linting errors, so we are going to just ignore them; I thought these were originally strings which is why I attempted to join them, but they are some form of lists

run futurize

2f5c112

started to debug tests; added generic test.ini stuff for ckan; fixed …

a8d2ffa

…plugin parameters

use ckan-dev db instead of production becaue postgis is not required …

b2564b1

…for this extension; started to fix py3 errors, but a lot of them stem from controller to blueprint issue, so trying to make fresh slate for that commit

converted controllers to blueprints, there were quite a few hiccups o…

6f7bfb7

…f things no longer existing and/or being rewritten, will try to see how the tests do..

fixed code reference to 'REDACTION_STROKE_REGEX'; fixed code referenc…

759ffed

…e to 'fanstatic' directory; fixed template reference to dashboard home (following example in 'ckanext-datagovtheme' ./ckanext/datagovtheme/templates/templates_2_8/header.html:46)

first attempt at upgrading tests; but there's an issue with pylons ev…

d3a7f92

…en though I'm not using it directly; the 'before_search' function in the plugin is causing an issue because there is no good way to verify a user is authorized right now

git was ignoring this file, it was supposed to be added as part of th…

b2fab47

…e controller to blueprint conversion

stop ignoring __init__.py

8386dd1

updated single test to new test framework; will do the rest increment…

00f0153

…ally; fixed lint; these changes will change a lot of the plugin functionality...

to simplify the upgrade, a least restrictive validation test is being…

aa72097

… taken which may cause incomplete data uploads; but until we know the proper way to update the validation, this was deemed the best approach

modify the routing of package creation/update function calls; will re…

32b7df4

…visit soon

got the second of the tests updated appropriately

d0b8ec6

fix test.ini location for circleci tests

c26060c

add usmetadata back into ckan plugins; accidentally removed

2eacf8c

upgraded test_package_update

1142256

install local requirements.txt file for circleci build since 'builtin…

46170cd

…s' is imported from future for py3 support

try to run nosetests on nose directory for circleci

8dfb8f8

updated nosetest import for path change

8ef8898

made db_utils function more readable

74933ae

upgraded test_package_parent_dataset

c0123ac

fixed blueprint routes

a7cbc9e

fixed import of request and reference to 'application_url'; updated t…

af62e82

…est_validate_dataset_action and added additional validation tests as referenced in TODO

updated test_validate_resource_action

4605da8

removed all of the debug print statements that I added and upgraded t…

28d4038

…he rest of the tests;... well... lint passes and the tests seem to work on CKAN 2.8.... moving on to CKAN 2.9 now? :s"

attempt to fix test.ini for github actions

6b6213d

nickumia-reisys added 6 commits August 3, 2021 00:38

tests: testFieldValidationContactEmail* works/upgraded for 2.8

5211510

tests: evaluated/updated the rest of unit_test.py; everything either …

a9a401b

…works in 2.8 or is commented out because of a schema change in the plugin; fixed lint

tests: everything passes in 2.9; this issue was only a logging type e…

b784834

…rror

clean: remove old circleci stuff

6af1d6e

new: updated js/css assets to webassets

173e7f1

new: updated templates from 'package' to 'dataset' or 'resource'

6a65cb0

nickumia-reisys added 2 commits August 3, 2021 02:38

doc: README

b25ead8

doc: ADR creation

6b6b1ba

nickumia-reisys added 5 commits August 3, 2021 15:31

new: updated schmema to reconcile validation issues

8f69b08

tests: fixed string_length_validation and removed tests that didn't m…

cd5296d

…ake sense

Merge branch 'feature/py3' of github.com:GSA/ckanext-usmetadata into …

699fa4f

…feature/py3

fixed lint ... should have known :(

9923c7f

new/tests: added info to attribute too long error indicating the limi…

5b373ec

…t for the field; fixed test errors

mogul requested a review from jbrown-xentity August 5, 2021 20:16

jbrown-xentity previously approved these changes Aug 6, 2021

View reviewed changes

nickumia-reisys added 3 commits August 6, 2021 14:58

docs: update TODO to describe desired functionality of the commented …

d1c10ca

…out tests

docs: update ADRs

14676e5

new: split requirements into real requirements and dev requirements

e9340ca

nickumia-reisys dismissed jbrown-xentity’s stale review via e9340ca August 6, 2021 19:19

nickumia-reisys requested a review from jbrown-xentity August 6, 2021 19:20

nickumia-reisys added 2 commits August 6, 2021 15:37

tests: install new dev-requirements.txt

031c869

tests: also add dev-requirements to github actions :/

98c2a48

jbrown-xentity approved these changes Aug 10, 2021

View reviewed changes

jbrown-xentity merged commit f28f00b into main Aug 10, 2021

jbrown-xentity deleted the feature/py3 branch August 10, 2021 16:28

nickumia-reisys mentioned this pull request Dec 25, 2021

Validating CKAN2.9 & Python3 framework with catalog GSA/data.gov#3401

Closed

4 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Feature/py3 #53

Feature/py3 #53

nickumia-reisys commented Jul 31, 2021

nickumia-reisys commented Aug 3, 2021

nickumia-reisys commented Aug 3, 2021

mogul commented Aug 3, 2021

nickumia-reisys commented Aug 5, 2021

jbrown-xentity left a comment

jbrown-xentity Aug 3, 2021

nickumia-reisys Aug 6, 2021

jbrown-xentity Aug 6, 2021

nickumia-reisys Aug 6, 2021

jbrown-xentity Aug 6, 2021

nickumia-reisys Aug 6, 2021

jbrown-xentity Aug 6, 2021

nickumia-reisys Aug 6, 2021

jbrown-xentity Aug 6, 2021

nickumia-reisys Aug 6, 2021

jbrown-xentity Aug 6, 2021

nickumia-reisys Aug 6, 2021

jbrown-xentity left a comment


		converted_data, errors = df.validate(data, schema)

		# TODO: schema approach was changed, so we don't care if it's missing right now


		### Consequences

		- Until the schema is updated properly, incomplete input data may cause unknown bugs throughout the system.


		### Consequences

		- The string_length_validator has known issues where sometimes it allows strings that are longer than allowed. However, as a unit test, the function works.

		@@ -1 +1,7 @@
		flake8

Feature/py3 #53

Feature/py3 #53

Conversation

nickumia-reisys commented Jul 31, 2021

nickumia-reisys commented Aug 3, 2021

nickumia-reisys commented Aug 3, 2021

mogul commented Aug 3, 2021

nickumia-reisys commented Aug 5, 2021

jbrown-xentity left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jbrown-xentity left a comment

Choose a reason for hiding this comment