-
Notifications
You must be signed in to change notification settings - Fork 94
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
CMR-9502: Has-granules-map cache to redis #2058
base: master
Are you sure you want to change the base?
Conversation
Codecov ReportAttention:
Additional details and impacted files@@ Coverage Diff @@
## master #2058 +/- ##
===========================================
- Coverage 58.12% 28.00% -30.13%
===========================================
Files 1042 1042
Lines 69529 69539 +10
Branches 1950 1176 -774
===========================================
- Hits 40413 19472 -20941
- Misses 27280 48893 +21613
+ Partials 1836 1174 -662 ☔ View full report in Codecov by Sentry. |
@@ -35,6 +35,7 @@ | |||
[cmr.metadata-db.config :as mdb-config] | |||
[cmr.metadata-db.system :as mdb-system] | |||
[cmr.metadata-db.services.util :as mdb-util] | |||
[cmr.search.services.query-execution.has-granules-results-feature :as has-granules-results-feature] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
you could say has-gran-rslt-feat, it's a bit long, but I don't mind either way. I love how people use longer names these days, but still remember fondly when you could shorting things up
@@ -46,6 +46,12 @@ | |||
(fn [concept-type query] | |||
[concept-type (qm/base-result-format query)])) | |||
|
|||
(defmethod concept-type+result-format->fields [:granule :xml] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I have not seen a + sign in a function name in our code base, have you seen another function do this? Obviously you can do it, but it stands out and I wonder what other developers think.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't see anything wrong, just a couple of questions, one open to everyone, but I don't see any reason to hold this ticket up.
UPDATE: THIS PR IS ON HOLD UNTIL UNBLOCKED BY REDIS FIXES. WILL WORK ON NEXT PI 24.2.0