Skip to content

Conversation

@jeromevdl
Copy link
Contributor

Issue #, if available:

Description of changes:

Add AppConfig in the parameter module.

Checklist

Breaking change checklist

RFC issue #:

  • Migration process documented
  • Implement warnings (if it can live side by side)

By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice.

@jeromevdl
Copy link
Contributor Author

I need to add the documentation.

@pull-request-size pull-request-size bot added size/XL and removed size/L labels Apr 8, 2022
- arn:aws:lambda:us-east-1:027255383542:layer:AWS-AppConfig-Extension:68
Environment:
Variables:
POWERTOOLS_APPCONFIG_EXTENSION: 'true'
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

From user experience perspective, is there a way we can build this intelligence within the AppConfig Provider? Like figuring it out from function config etc ?

@heitorlessa
Copy link
Contributor

heitorlessa commented Apr 9, 2022 via email

@jeromevdl
Copy link
Contributor Author

I read an env bar for Extension. Is there a RFC to learn more about this behaviour? For Python, we offer normal integration with AppConfig via the Parameters utility. I couldn’t find a related issue or RFC in the PR hence the ask. Thanks!

Not sure to understand your point @heitorlessa . The aim of this PR is actually to add AppConfig to the Parameters module. There's no RFC, or whatever, just wanted to provide this feature which is available in python. I'm currently looking for another way to detect if the extension is available rather than using this env var. If you know how, let me know...

}
```

### Using the AppConfig Extension for Lambda
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is the part I was referring to @jeromevdl - we don't support this in Python.

That's why I was asking for a RFC to understand the behaviour so it could be assessed for other languages later.

Does that make sense?

@heitorlessa
Copy link
Contributor

I read an env bar for Extension. Is there a RFC to learn more about this behaviour? For Python, we offer normal integration with AppConfig via the Parameters utility. I couldn’t find a related issue or RFC in the PR hence the ask. Thanks!

Not sure to understand your point @heitorlessa . The aim of this PR is actually to add AppConfig to the Parameters module. There's no RFC, or whatever, just wanted to provide this feature which is available in python. I'm currently looking for another way to detect if the extension is available rather than using this env var. If you know how, let me know...

Np, I've added a comment on the exact line that suggests Powertools supports AppConfig Lambda Extension - we don't support that in Python today, hence the ask for a RFC.

For your question, AppConfig Lambda Extension runs locally on localhost:2772 - if the socket is open the customer is running AppConfig extension.

Under "How it works"

https://docs.aws.amazon.com/appconfig/latest/userguide/appconfig-integration-lambda-extensions.html

@jeromevdl
Copy link
Contributor Author

@heitorlessa @pankajagrawal16
==> #827

After writing this RFC, I must admit I'm not so sure it's a good idea to provide this option.

@pankajagrawal16
Copy link
Contributor

pankajagrawal16 commented Apr 13, 2022

@heitorlessa @pankajagrawal16 ==> #827

After writing this RFC, I must admit I'm not so sure it's a good idea to provide this option.

Looking at drawbacks you mentioned in RFC, I would agree as well. So in my eyes, it will add value to support extension bit if we can technically make DX experience like if users adds app config extension to the function and if they were using this module as well, the functionality should just seamlessly switch to get data from cached extension environment. (without need to do any extra config etc via env variable and caching bit within utility)

@heitorlessa
Copy link
Contributor

@heitorlessa @pankajagrawal16 ==> #827

After writing this RFC, I must admit I'm not so sure it's a good idea to provide this option.

this is the best part of writing, it helps us put things in perspective and to perhaps see it from a different angle. I agree too it's best to drop the Lambda Extension part for now.

Future wise, this gives you a better chance to hear customer demand, and work with customers that do use the Extension to confirm whether (1) this would be beneficial, and (2) what a good DX looks like (trade-offs included).

@jeromevdl jeromevdl requested a review from pankajagrawal16 May 30, 2022 09:03
@jeromevdl
Copy link
Contributor Author

Closing (as per the above comments)

@jeromevdl jeromevdl closed this Feb 21, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants