Skip to content

wire session factory and partial flyweight optional support #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

Open
wants to merge 1 commit into
base: develop
Choose a base branch
from

Conversation

kapilt
Copy link
Contributor

@kapilt kapilt commented May 31, 2016

not sure wrt to approach here, i've tried a separate query executor class against extant meta models as well, which might be a little nicer, but wanted to try and see what it looked like to modify extant for library usage.

This unifies the client creation behind a passed object (session_factory) through most of the tree. cloudwatch usage is made lazy, as client construction does require reading model files and is noticable in lambda. there's partial support for choosing between flyweight resource classes and raw dictionaries.

mostly to start a discussion, i'll post a query branch later this week for comparison.

@kapilt
Copy link
Contributor Author

kapilt commented Jun 3, 2016

wrt to query model, https://github.com/capitalone/cloud-custodian/blob/master/c7n/query.py#L81

that's basically just a base layer reimplementing the query logic embedded in resource.py but also just working with the raw data structs

@avram
Copy link
Contributor

avram commented Sep 3, 2016

@garnaat Thoughts on this approach vs #86?

@garnaat
Copy link
Contributor

garnaat commented Sep 4, 2016

I like this basic approach. I'm going to play around with it a bit this weekend and may have more comments.

@kapilt
Copy link
Contributor Author

kapilt commented Oct 4, 2016

fwiw, i've been expanding out use of skew models to include aws config support. also considering expansion to include create, modify, delete events.

@walterheck
Copy link

Any chance this can be amended and merged? Would be really nice to be able to use this..

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants