Keep AccountTypeDef to avoid DescribeAccount calls

[iainelder]

In this design I haven't eliminated the use of DescribeAccount completely. It may still be needed to fill in the details for IDs captured via target_ids. But now all the organizations API calls are in the CoveHostAccount class, where they make most sense, and the active_cove_session function concerns itself only with assuming the role in the target account.

I've marked this as a draft because there's two things I'm unhappy with.

It seems to have introduced a memory leak. I may need to dig out the old test code from #20 to confirm that with more rigor.

The change of the internal structure from a list of IDs to a dict of IDs to AccountTypeDefs means the elegant hack used in #37 to repeat a single target account to simulate a big org doesn't work any more. I think the testing in #20 used the same technique, so I'll need to find another way to support that.

Any thoughts on how to solve these problems?

[connelldave]

I'm curious if Moto can reproduce the memory leak with an emulated 5000 individual accounts rather than the trick around one ID repeated - with the new functionality for regions, we could test org membership * regions available too I think.

What would be really cool is capturing that in pytest so we can look for this regression in future changes - I very quickly scrolled the diff and didn't see anything obvious that would reintroduce that garbage collection issue.

In general though - I'm getting married next week so I'm very much unlikely going to be able to look at this properly for a couple of weeks, but grateful for you picking it up! I'm hoping to invest a bunch of time into this project in the back end of this year as it's solving a problem I'm seeing colleagues face a lot too (mainly discovery around unknown AWS orgs, rather than making changes) so I'll be keen to learn from your experience :)

[iainelder]

Congratulations! Enjoy that and don't worry at all about botocove for a few weeks :-)

I'll update here if I find a way to solve the leak in the meantime.

[iainelder]

It seems to have introduced a memory leak.

I don't think this change is to blame for that. I think rather it's because of a calling style that I started using in my quick-and-dirty scripts. Today I observed similar behavior using version 1.6.1.

Using the decorator on a function like this is fine. Memory use stays constant.

@cove()

Using the decorator on a function like this is leaky. Memory use increases as each account is processed.

@cove(
    assuming_session=(
        Session(profile_name=os.environ["ORG_MGMT"], region_name="eu-west-1")
    )
)

I'll make a new issue to track that once I have a repro and some data to share.

I'll keep this issue on draft till then.

What would be really cool is capturing that in pytest so we can look for this regression in future changes.

Yes, an automatic stress test would make me more confident that it will work as we expect at scale. It would be really convenient to reproduce the leaky behavior using just moto. I suspect we might have to use server mode rather than decorator mode so that boto3 behaves more like it would against a real AWS account.

[iainelder]

Closing this attempt because I found a better way to do it in #41.