#1206: add async to aws:s3 and aws:ecr #1192

ramonpetgrave64 · 2023-06-23T23:19:51Z

Add some helper functions for refactoring existing modules to be more async and use them for aws:s3 and aws:ecr.
In a test on one of our larger aws accounts, we've seen ~90% reduction in sync time for s3 and ecr.

to_async wraps a regular synchronous function so that it can be used within async functions in a non-blocking fashion
so_sync
1. takes a series are Awaitables (e.g., container objects returned when invoking async functions)
2. schedules them all to be run simultaneously
3. blocks until they are all finished
4. returns all the results in the same order

There is a caveat when running these functions from a Jupyter notebook.
You must do a workaround, but homefully in future versions of asyncio this will not be needed

    # import nest_asyncio
    # nest_asyncio.apply()

Testing

Existing unit tests mostly cover these changes, but I also did some manual testing against a real AWS account.

cleanup

achantavy · 2023-07-12T18:27:55Z

cartography/util.py

+    return asyncio.get_event_loop().run_in_executor(None, call)
+
+
+def to_sync(*awaitables: Awaitable[Any]) -> Any:


Suggested change

def to_sync(*awaitables: Awaitable[Any]) -> Any:

def to_synchronous(*awaitables: Awaitable[Any]) -> Any:

since the word "sync" is used a lot in the project

achantavy · 2023-07-12T18:28:02Z

cartography/util.py

@@ -297,3 +301,31 @@ def batch(items: Iterable, size: int = DEFAULT_BATCH_SIZE) -> List[List]:
        items[i: i + size]
        for i in range(0, len(items), size)
    ]
+
+
+def to_async(func: Callable, *args: Any, **kwargs: Any) -> asyncio.Future:


Suggested change

def to_async(func: Callable, *args: Any, **kwargs: Any) -> asyncio.Future:

def to_asynchronous(func: Callable, *args: Any, **kwargs: Any) -> asyncio.Future:

achantavy · 2023-07-13T21:13:31Z

cartography/intel/aws/ecr.py

-        for repo in repositories:
-            repo_image_obj = get_ecr_repository_images(boto3_session, region, repo['repositoryName'])
+
+        async def async_get_images(repo: Dict[str, Any]) -> None:


For someone like me who does not know how async works, can you briefly describe what's going on here? E.g. how many calls to get_ecr_repository_images are happening in parallel, is it configurable, etc?

~~10 at most becuase of the default threadpool size. checking to see if we can make it configurable~~

Update.
There was a time when the default was 10. but here are docs
https://docs.python.org/3.8/library/concurrent.futures.html#concurrent.futures.ThreadPoolExecutor

Changed in version 3.8: Default value of max_workers is changed to min(32, os.cpu_count() + 4). This default value preserves at least 5 workers for I/O bound tasks. It utilizes at most 32 CPU cores for CPU bound tasks which release the GIL. And it avoids using very large resources implicitly on many-core machines.

I think it's better to leave it at the default. On an M1 with 12 cores, it doesn't get noticeably faster than the default, even with max_workers=1000.

achantavy · 2023-07-13T21:40:52Z

Also: I just played with this and it is indeed very slick 👍

This PR fix two bugs in current version : - #1189 - #1182

Adds docs on - discovering what other nodes are connected to a given node type - discovering what node properties are present on a given node type Updates docs on - roadmap link - making syncmetadata docs more discoverable

…iple intel modules (#1214) See #1210 for full context. #1154 tried to solve this problem by updating the querybuilder but this was too complex and would not generalize well. This solution is simpler where we use different property classes for each API response so that we don't overwrite properties on a node set by another sync job. This PR can be reviewed commit-by-commit: - c0d9ac4 shows a repro of the error with a failing integration test. - facb63b shows the solution using multiple classes. --------- Co-authored-by: Ramon Petgrave <[email protected]>

It's possible for neo4j sessions `read_transaction` in `get_state` to return an empty list in the drift detection module. This PR ensures that there are entries before referencing index 0. ``` File "/code/venvs/venv/lib/python3.8/site-packages/cartography/driftdetect/get_states.py", line 123, in get_query_state get_state(session, state) File "/code/venvs/venv/lib/python3.8/site-packages/cartography/driftdetect/get_states.py", line 148, in get_state state.properties = list(new_results[0].keys()) IndexError: list index out of range ```

Instruct how we plan to use the Discussions feature

cartography/intel/aws/ecr.py

achantavy · 2023-08-04T16:14:53Z

cartography/util.py

+
+    Calls are also wrapped within a backoff decorator to handle throttling errors.
+
+    example:


Good that you included an example, but since this is going to be a core function, I think we should have detailed docs on what each of the params is.

cartography/util.py

achantavy · 2023-08-04T16:17:08Z

cartography/util.py

+    def wrapper(*args: Any, **kwargs: Any) -> R:
+        try:
+            return func(*args, **kwargs)
+        except botocore.exceptions.ClientError as error:


This func is located in cartography.util. If we decide to use this for other sync jobs (non-AWS), will we add all of the different exception types here too, like will GCP exceptions be here too? Might end up having a lot of excepts.

You're right. I've refactored to another is_throttling_exception() function

achantavy · 2023-08-04T16:18:41Z

cartography/util.py

+    # nest_asyncio.apply()
+    '''
+    CartographyThrottlingException = type('CartographyThrottlingException', (Exception,), {})
+    throttling_error_codes = ['LimitExceededException', 'Throttling']


This set of functions is located in cartography.util. Other libraries might use different exception names, so it might get unwieldy to have AWS, GCP, Azure names for throttling exceptions all in one place. I wonder if it might make sense to have this in cartography.aws.util (or whatever the equivalent is) first until we have an example of doing this with another module.

achantavy · 2023-08-04T16:18:59Z

cartography/util.py

+    Waits for the Awaitable(s) to complete and returns their result(s).
+    See https://docs.python.org/3.8/library/asyncio-task.html#asyncio-awaitables
+
+    example:


Same thing about explicitly documenting the parameters.

added as the normal :param : spec

achantavy · 2023-08-22T21:04:26Z

cartography/util.py

+    CartographyThrottlingException = type('CartographyThrottlingException', (Exception,), {})
+
+    @wraps(func)
+    def wrapper(*args: Any, **kwargs: Any) -> R:


[nonblock][question] What's the typevar R do?

It binds the return type of func so that mypy can understand that the wrapped function is supposed to still return type R.

achantavy

Awesome thank you!

…ncf#1192) Add some helper functions for refactoring existing modules to be more async and use them for aws:s3 and aws:ecr. In a test on one of our larger aws accounts, we've seen ~90% reduction in sync time for s3 and ecr. - `to_async` wraps a regular synchronous function so that it can be used within async functions in a non-blocking fashion - `so_sync` 1. takes a series are [Awaitables](https://docs.python.org/3.8/library/asyncio-task.html#asyncio-awaitables) (e.g., container objects returned when invoking async functions) 2. schedules them all to be run simultaneously 3. blocks until they are all finished 4. returns all the results in the same order There is a caveat when running these functions from a Jupyter notebook. You must do a workaround, but homefully in future versions of asyncio this will not be needed ``` # import nest_asyncio # nest_asyncio.apply() ``` ### Testing Existing unit tests mostly cover these changes, but I also did some manual testing against a real AWS account.

ramonpetgrave64 added 2 commits June 23, 2023 15:34

init

55a1545

cleanup

note about nest_asyncio

3aa19af

ramonpetgrave64 changed the title ~~async to aws:s3 and aws:ecr~~ add async to aws:s3 and aws:ecr Jun 23, 2023

ramonpetgrave64 mentioned this pull request Jul 6, 2023

AWS: Make Fetches Concurrent #1197

Closed

achantavy reviewed Jul 12, 2023

View reviewed changes

achantavy reviewed Jul 13, 2023

View reviewed changes

achantavy changed the title ~~add async to aws:s3 and aws:ecr~~ #1206: add async to aws:s3 and aws:ecr Jul 14, 2023

jychp and others added 12 commits August 3, 2023 14:47

Hotfix (#1190)

04d7fe9

This PR fix two bugs in current version : - #1189 - #1182

0.81.0 (#1191)

17f1288

Set to None the gsuite token instead of github one (#1204)

362cfd3

0.82.0.dev1

94bfabc

'0.82.0.dev2'

38a7a40

Update usage docs and roadmap links (#1196)

9feb4fd

Adds docs on - discovering what other nodes are connected to a given node type - discovering what node properties are present on a given node type Updates docs on - roadmap link - making syncmetadata docs more discoverable

Add contributing guidelines for issues (#1226)

f889b53

Instruct how we plan to use the Discussions feature

add throttling and typing

40b075a

add boto3_session fixture with stubbed clients

da5c68d

cleanup

4c89a35

ramonpetgrave64 force-pushed the ramonpetgrave64-s3-async branch from c2cc178 to 4c89a35 Compare August 3, 2023 18:51

rename

c8273f1

ramonpetgrave64 force-pushed the ramonpetgrave64-s3-async branch from 6c9afd1 to c8273f1 Compare August 3, 2023 20:24

ramonpetgrave64 added 2 commits August 3, 2023 16:25

Merge branch 'master' into ramonpetgrave64-s3-async

bdda3d1

Update setup.py

f8eadd0

achantavy requested changes Aug 4, 2023

View reviewed changes

ramonpetgrave64 added 3 commits August 8, 2023 11:28

Merge branch 'master' into ramonpetgrave64-s3-async

8257fe4

refactor, add examples

a92b20e

typing

92c72ad

ramonpetgrave64 and others added 2 commits August 8, 2023 12:41

whitespace

bdc664f

Merge branch 'master' into ramonpetgrave64-s3-async

a06d145

achantavy reviewed Aug 22, 2023

View reviewed changes

ramonpetgrave64 added 5 commits August 22, 2023 17:15

add :param : docs

4ac4dfa

Update util.py

128ef7a

lint

8f770f7

lint

1ad7a8f

lint

4edaefb

achantavy approved these changes Aug 22, 2023

View reviewed changes

achantavy merged commit af0b60f into master Aug 22, 2023
4 checks passed

achantavy deleted the ramonpetgrave64-s3-async branch August 22, 2023 22:14

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

#1206: add async to aws:s3 and aws:ecr #1192

#1206: add async to aws:s3 and aws:ecr #1192

ramonpetgrave64 commented Jun 23, 2023 •

edited

Loading

achantavy Jul 12, 2023 •

edited

Loading

achantavy Jul 12, 2023

ramonpetgrave64 Aug 3, 2023

achantavy Jul 13, 2023

ramonpetgrave64 Aug 3, 2023 •

edited

Loading

achantavy commented Jul 13, 2023

achantavy Aug 4, 2023

ramonpetgrave64 Aug 8, 2023

achantavy Aug 4, 2023

ramonpetgrave64 Aug 8, 2023

achantavy Aug 4, 2023

achantavy Aug 4, 2023

ramonpetgrave64 Aug 8, 2023

ramonpetgrave64 Aug 22, 2023

achantavy Aug 22, 2023

ramonpetgrave64 Aug 22, 2023

achantavy left a comment

		return asyncio.get_event_loop().run_in_executor(None, call)


		def to_sync(*awaitables: Awaitable[Any]) -> Any:

	def to_sync(*awaitables: Awaitable[Any]) -> Any:
	def to_synchronous(*awaitables: Awaitable[Any]) -> Any:

	def to_async(func: Callable, args: Any, *kwargs: Any) -> asyncio.Future:
	def to_asynchronous(func: Callable, args: Any, *kwargs: Any) -> asyncio.Future:


		Calls are also wrapped within a backoff decorator to handle throttling errors.

		example:

#1206: add async to aws:s3 and aws:ecr #1192

#1206: add async to aws:s3 and aws:ecr #1192

Conversation

ramonpetgrave64 commented Jun 23, 2023 • edited Loading

Testing

achantavy Jul 12, 2023 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ramonpetgrave64 Aug 3, 2023 • edited Loading

Choose a reason for hiding this comment

achantavy commented Jul 13, 2023

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

achantavy left a comment

Choose a reason for hiding this comment

ramonpetgrave64 commented Jun 23, 2023 •

edited

Loading

achantavy Jul 12, 2023 •

edited

Loading

ramonpetgrave64 Aug 3, 2023 •

edited

Loading