-
Notifications
You must be signed in to change notification settings - Fork 1
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
MODLD-601: LCCN deduplication configuration setting #59
Conversation
src/main/resources/application.yml
Outdated
@@ -82,6 +82,7 @@ mod-linked-data: | |||
cache: | |||
ttl: | |||
spec-rules: ${CACHE_TTL_SPEC_RULES:18000000} | |||
settings-entries: ${CACHE_TTL_SETTINGS_ENTRIES:18000000} |
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.
spring.cache.type=none can be used to disable all caches in karate tests
@pkjacob
src/main/resources/application.yml
Outdated
@@ -82,6 +82,7 @@ mod-linked-data: | |||
cache: | |||
ttl: | |||
spec-rules: ${CACHE_TTL_SPEC_RULES:18000000} | |||
settings-entries: ${CACHE_TTL_SETTINGS_ENTRIES:18000000} |
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.
CACHE_TTL_SETTINGS_ENTRIES
should be added to README and ModuleDescriptor files
try { | ||
return !settingsService.isSettingEnabled(DUPLICATE_CHECK_SCOPE, DUPLICATE_CHECK_KEY, DUPLICATE_CHECK_PROPERTY); | ||
} catch (Exception e) { | ||
return true; |
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.
maybe add some log here?
@@ -81,8 +81,8 @@ mod-linked-data: | |||
instance-ingress: ${KAFKA_INVENTORY_INSTANCE_INGRESS_EVENT_TOPIC:inventory.instance_ingress} | |||
cache: | |||
ttl: | |||
spec-rules: ${CACHE_TTL_SPEC_RULES:18000000} | |||
settings-entries: ${CACHE_TTL_SETTINGS_ENTRIES:18000000} | |||
spec-rules: ${CACHE_TTL_SPEC_RULES_MS:18000000} |
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.
👍
|
||
public interface SettingsService { | ||
|
||
Optional<SettingsItem> getSetting(String scope, String key); |
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.
Any need to expose this method in the interface? I believe only isSettingEnabled
need to be exposed in the interface and getSetting
can be a private method in the impl class.
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.
just wanted to make it public for future usages, but yes, we ain't gonna need it now
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.
changed to private
try { | ||
return !settingsService.isSettingEnabled(DUPLICATE_CHECK_SCOPE, DUPLICATE_CHECK_KEY, DUPLICATE_CHECK_PROPERTY); | ||
} catch (Exception e) { | ||
log.warn("An exception occurred during settings service call", e); |
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.
decided to change it to 'error'
MODLD-601: Review fixes MODLD-601: Review fixes MODLD-601: Change log level for settings service outage
37e7bf8
to
4d1c0c7
Compare
Quality Gate passedIssues Measures |
No description provided.