-
Notifications
You must be signed in to change notification settings - Fork 128
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
Jormungandr: Cache Co2 emission value instead of pb object #3965
Conversation
Kudos, SonarCloud Quality Gate passed! |
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 believe this can be fixed differently in a much simpler way.
current_app.config[str('MEMORY_CACHE_CONFIGURATION')].get(str('TIMEOUT_AUTHENTICATION'), 30) | ||
) | ||
@cache.memoize(current_app.config[str('CACHE_CONFIGURATION')].get(str('TIMEOUT_AUTHENTICATION'), 300)) | ||
def inner(mode_str): |
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.
Instead of using this convoluted solution of an inner function, to ignore the request_id
from the cache key, @cache.memoize
has an option to ignore some function parameters with args_to_ignore
(see documentation).
So the PR could only be
@memory_cache.memoize(
current_app.config[str('MEMORY_CACHE_CONFIGURATION')].get(str('TIMEOUT_AUTHENTICATION'), 30),
args_to_ignore=["request_id"],
)
@cache.memoize(
current_app.config[str('CACHE_CONFIGURATION')].get(str('TIMEOUT_AUTHENTICATION'), 300),
args_to_ignore=["request_id"],
)
s.co2_emission.CopyFrom(co2_emission) | ||
co2_emission = instance.georef.get_car_co2_emission(request_id) | ||
pb_co2_emission = s.co2_emission | ||
pb_co2_emission.value = (co2_emission * s.street_network.length) / 1000.0 if co2_emission > 0 else 0 |
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'm not sure that the if co2_emission > 0 else 0
is useful. In case co2_emission == 0
, the calculation will already be 0. Can co2_emission
be negative?
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.
This will avoid negative value
Two problems occurred while caching pb object Co2Emission
A little adjustment is made to pass request_id without including it's value as key while caching.
For more details: https://navitia.atlassian.net/browse/NAV-1667