CMR-11242: Adding Toggle for Granule collections Reference validation#2446
CMR-11242: Adding Toggle for Granule collections Reference validation#2446eudoroolivares2016 wants to merge 19 commits into
Conversation
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## master #2446 +/- ##
==========================================
- Coverage 58.20% 58.04% -0.17%
==========================================
Files 1069 1068 -1
Lines 74278 74148 -130
Branches 2166 2160 -6
==========================================
- Hits 43233 43036 -197
- Misses 29028 29093 +65
- Partials 2017 2019 +2 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
| (let [parsed-dif (c/parse-collection all-fields-collection-xml)] | ||
| (is (empty? (v/validate-collection parsed-dif)))))) |
There was a problem hiding this comment.
This was the only place that c/parse-collection was called so its not actually being used in production
| (v/validate (cons vv/variable-validations additional-validations) | ||
| variable)))) | ||
|
|
||
| (defn validate-variable-warnings |
There was a problem hiding this comment.
This wasn't being used at all anywhere
There was a problem hiding this comment.
looks like it is used in umm-spec-lib/src/cmr/umm_spec/validation but it looks like perhaps that func is not being used anywhere either...
There was a problem hiding this comment.
I'm not seeing that in here but, I did notice earlier that this is kinda dead code umm-spec-lib/src/cmr/umm_spec/validation/umm_spec_variable_validation.clj we use it in prod but, I mean its just a stub
| ["<example>data</example>"] | ||
| "<example>data</example>"))) | ||
|
|
||
| (deftest format-and-contextualize-warnings-existing-errors-test |
There was a problem hiding this comment.
when you build in bamboo, make sure this test is actually executed and you see them in the logs
There was a problem hiding this comment.
So its not in the log precisely but, it did run all the unit tests in cmr-ingest-app on the log so I think thats sufficient
61e2845 to
57b28c4
Compare
57b28c4 to
70ec416
Compare
| (constantly [parent-collection-concept | ||
| collection])))) | ||
|
|
||
| ;; Declared because of a circular dependency in `validate granule` |
There was a problem hiding this comment.
double check probably remove comment
| (info (format "Validating granule %s from client %s" (api-core/concept->loggable-string concept) | ||
| (:client-id request-context))) | ||
| (ingest/validate-granule request-context concept) | ||
| {:status 200})) |
There was a problem hiding this comment.
I am not sure that the validate endpoint is going to actually return your new warnings, because as you can see here the actual function always returns 200 because it expects the call to the ingest/validate-granule to raise if there is errors.
|
Approving assuming that it's ok that validate endpoint won't return new granule warnings as ingest endpoint does |
Overview
What is the objective?
This adds a feature toggle which when set to
falsewill allow ingesting granules even if the references between keywords (namely, projects, platforms, instruments, instrument characterists , and operational modes). Instead, those will be returns as warnings from ingest responseWhat are the changes?
Major change: Creating the feature toggle in the
commonas it needed to be used in bothingestand theumm-lib. Threading that up from the validation so that warnings can be propagated down to the end user response.Minor Changes:
Creating the feature toggle in the
commonas it needed to be used in bothingestand theumm-libthere were some additional validation which was dead-code that I am removing. The linter picked up a few changes that I allowed it to leave in there namely spacing. Changing imports to just make this more consistent.What areas of the application does this impact?
Granule ingest and granule validation
Required Checklist
Additional Checklist