CMR-11237: Validate Ingest and invoke Metadata Fix Service - #2471
CMR-11237: Validate Ingest and invoke Metadata Fix Service#2471eudoroolivares2016 wants to merge 20 commits into
Conversation
eudoroolivares2016
changed the title
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## master #2471 +/- ##
==========================================
- Coverage 57.93% 57.91% -0.03%
==========================================
Files 1073 1074 +1
Lines 74658 74699 +41
Branches 2175 2173 -2
==========================================
+ Hits 43251 43259 +8
- Misses 29379 29408 +29
- Partials 2028 2032 +4 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
…t this is going to
4dfd803 to
89ca410
Compare
| existing-errors (v/umm-spec-validate-collection | ||
| sanitized-collection sanitized-prev-collection validation-options context false) | ||
| collection-schema-warnings (v/validate-collection-umm-spec-schema collection validation-options) | ||
| ;; Return warnings for validation errors on collection without checking if they are |
There was a problem hiding this comment.
Is this an incomplete sentence?
There was a problem hiding this comment.
Fixed comment was from main got dropped along the way by mistake
| (e/map->PathErrors | ||
| {:path field-path | ||
| :errors (mapv (partial v/create-error-message | ||
| {:path field-path |
| (c/map->RangeDateTimeType | ||
| {:BeginningDateTime begin-date-time | ||
| :EndingDateTime end-date-time}))) | ||
| {:BeginningDateTime begin-date-time |
There was a problem hiding this comment.
Its all coming out of https://github.com/weavejester/cljfmt
| concept | ||
| validation-options) | ||
| (let [{:keys [concept warnings existing-errors has-keyword-error?]} | ||
| (validate-and-prepare-collection context conceptvalidation-options) |
There was a problem hiding this comment.
conceptvalidation-options ? Is this a typo? I don't think this symbol exists. Does this compile? I think you meant to put concept validation-options. Moreso concerned about whether this code was tested since this typo does not look like it compiles
There was a problem hiding this comment.
Yea I just updated that a second ago by mistake fixed
| [clojure.test :refer :all] | ||
| [cmr.ingest.services.ingest-service.collection :as collection])) | ||
|
|
||
| (deftest should-notify-kms?-test |
There was a problem hiding this comment.
Could consider adding tests that verify more behavior outside should-notify-kms? in isolation. For example verifying that if mdb/save-concept throws then KMS is not notified, that the notification receives the concept ID returned by mdb/save-concept, that no notificaiton occurs when the relevant warnings/errors are absent, etc. I'll DM you an example, up to you if it's worth it
5 participants
Overview
What is the objective?
This addresses an issue with the collection metadata that prevents the metadata-fixer service (a KMS module to update collection conceptId keywords on keyword updates) from being fully effective. There are some collections where ingests with the old keywords have to be allowed to ingest so those updates don't get blocked. This is a problem because if the metadata fixer service has updated the collection a subsequent ingest with the flag set to false will just override that. This PR when that condition is met will make a request from CMR to that new KMS service which in turn will ingest a new revision of the collection but, with the correct keyword
What are the changes?
Threading through the coll ingest a new bool for keyword-errors. This denotes cases where this would have caused an ingest error but, of course with the keyword flag in place may be allowed to ingest. After we get a conceptId back from the metadatadb we send that off to the kms fixer service. This is done using
asyc/gosimilar to how we reconcile providers with thecmr-orderingservice. That is a fire-and-forget so it won't slow down ingest. In transmit lib I've added a method to send this to the KMS which sits in the CMR load balancer and I'm leaving some rich comments for easily testing the integration Since we can't fully test that end to end locally because KMS isn't spun up by the REPL.All formating is done just using the
lein cljfmttool. I've split out variable validation just to keep the namespaces from getting longer than neededWhat areas of the application does this impact?
Collection ingest amd validation
Required Checklist
Additional Checklist