Skip to content
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

Feature: Parameters: add support for manual value refresh (WIP) #131

Closed
nmoutschen opened this issue Aug 24, 2020 · 7 comments
Closed

Feature: Parameters: add support for manual value refresh (WIP) #131

nmoutschen opened this issue Aug 24, 2020 · 7 comments
Labels
feature-request feature request good first issue Good for newcomers

Comments

@nmoutschen
Copy link
Contributor

Is your feature request related to a problem? Please describe.

Describe the solution you'd like

This would add a method to the parameter provider to force-refresh a parameter value. This could be done through a flag.

Use cases: username/password that have expired and need to be refreshed.

Describe alternatives you've considered

Additional context

@nmoutschen nmoutschen added triage Pending triage from maintainers feature-request feature request labels Aug 24, 2020
@heitorlessa heitorlessa removed the triage Pending triage from maintainers label Aug 30, 2020
@heitorlessa heitorlessa changed the title Parameters: add support for manual value refresh (WIP) Feature: Parameters: add support for manual value refresh (WIP) Aug 30, 2020
@heitorlessa heitorlessa added the good first issue Good for newcomers label Oct 24, 2020
@michaelbrewer
Copy link
Contributor

@nmoutschen @heitorlessa would this be via some kind of exception handler that picks up when the credentials failed?

@michaelbrewer
Copy link
Contributor

@nmoutschen a version of this is in this PR : #341

UX might be better for a decorator.

@heitorlessa
Copy link
Contributor

@nmoutschen could you expand on what you meant with this feature request so we can get it added in Wednesday's release?

Do you mean a parameter like what @michaelbrewer implemented on a per get_parameter call, or an explicit method to refresh what's already in the cache e.g. .refresh_parameter(name='/dev/service/parameter')?

If it's the former, @michaelbrewer I'd suggest a more explicit name like ignore_cache as that's what it seems to be happening in the implementation.

@nmoutschen
Copy link
Contributor Author

Hey @michaelbrewer & @heitorlessa!

I didn't have a specific implementation in mind and thought of 2 possibilities: pass a flag to the get_parameter call, or manually flush a specific cache value. Overall, I think @michaelbrewer's implementation is simpler (same function, with just an additional parameter).

From a name point of view, ignore_cache would imply that the parameter wouldn't be stored in local cache, so I personally prefer force_update. force_fetch is good too.

@heitorlessa
Copy link
Contributor

Thanks @nmoutschen that makes sense. force_fetch in this case makes more sense -- @michaelbrewer could you do a quick refactor on that PR with that and we can merge ahead of Wed's release?

@michaelbrewer
Copy link
Contributor

@heitorlessa i have made the changes just thinking of a good example for the docs, here it was i have so far:

import os
from aws_lambda_powertools.utilities import parameters


class InvalidCredentials(Exception):
    """Raised when client get an invalid credentials error"""


class Client:
    password = None

    def configure(self, password):
        self.password = password

    def read_record(self):
        """Might raise an InvalidCredentials exception """


DB_PASS_PARAM = os.environ["DB_PASS_PARAM"]
db_client = Client()
db_client.configure(parameters.get_parameter(DB_PASS_PARAM))


def refresh_db_password():
    db_client.configure(parameters.get_parameter(DB_PASS_PARAM, force_fetch=True))


def read_record(is_retry=False):
    try:
        return db_client.read_record()
    except InvalidCredentials:
        if not is_retry:  # avoid infinite recursion
            refresh_db_password()  # force parameter refresh
            return read_record(is_retry=True)


def handler(event, context):
    return {"record": read_record()}

@heitorlessa heitorlessa added pending-release Fix or implementation already in dev waiting to be released and removed pending-release Fix or implementation already in dev waiting to be released labels Mar 17, 2021
@heitorlessa
Copy link
Contributor

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
feature-request feature request good first issue Good for newcomers
Projects
Development

No branches or pull requests

3 participants