Added new APIv2 endpoints that allow advanced searches and included pagination functionality #47

alannix-lw · 2021-12-23T02:03:53Z

No description provided.

laceworksdk/http_session.py

hazedav · 2021-12-31T01:29:15Z

laceworksdk/api/cloud_activities.py

@@ -29,49 +32,84 @@ def __init__(self, session):
    def get(self,


Is the idea for paginated apis that get() will always return "all" and that if you actually want to work with pagination you would use a raw get_pages()? Meaning we're not actually exposing iterables at the api layer?

This is one of the things that I'm wrestling with on this update. My dream was to give any existing integrations all pages of data for the price of "on the house," but due to the lack of properly utilizing a generator, I realize that there may be some significant performance and resource consumption issues related to that. Would love some feedback on whether we should leave get alone and simply introduce new get_pages and items/get_items methods for pagination.

I don't fully understand the backwards compatibility requirements as it pertains to whether or not we're adding paging to things we've already established that get() would return everything...

One approach could be that get() would be a generator and all() would return everything. For things that aren't paginated we could just alias get->all.

Otherwise you could establish get() as the thing that returns everything and for paginated resource introduce a get_pages() which would also seem to align with the library.

hazedav · 2021-12-31T01:31:18Z

laceworksdk/http_session.py


-        logger.info(f"PATCH request to URI: {uri}")
-        logger.debug(f"PATCH request data:\n{data}")
+    def get_pages(self, uri, params=None, **kwargs):


Are there any pagination request settings (i.e. max items per page) here that we would want to promote, or should this simply be part of params?

Waiting for final documentation on this, but it looks like max items and payload size limits are "future features": https://docs.google.com/document/d/12z5Q7byMJQEoUFS-eLGspyPN-5eMC3jm/edit#heading=h.dfxjw4abp07x

well, we can also add in a max_items here and just have a simple counter.. that is not here, but in hte get_items function, and then exit the generator once you reach that max number.

kiddinn

few comments

laceworksdk/api/__init__.py

laceworksdk/api/base_endpoint.py

laceworksdk/api/v2/cloud_activities.py

kiddinn · 2022-01-04T10:30:01Z

laceworksdk/http_session.py

-        return response
+            try:
+                next_page = response.json()['paging']['urls']['nextPage']
+            except Exception:


this is a pretty broad exception handling here... can y8ou narrow that down?

also better to split this up a bit

at least to...

response_json = response.json() next_page = response_json.get("paging", {}).get("urls", {}).get("nextPage")

That way you at least don't get KeyError there, etc. (also single vs double quote for consistency in the code base)

you marked this as resolved, yet I still see the same code, with a except Exception here

(that is to say, you did adress part of the comment, just not all of it, and not the broad exception handling aspect of it)

laceworksdk/http_session.py

kiddinn · 2022-01-04T10:37:21Z

laceworksdk/http_session.py


-        logger.info(f"PATCH request to URI: {uri}")
-        logger.debug(f"PATCH request data:\n{data}")
+    def get_pages(self, uri, params=None, **kwargs):


well, we can also add in a max_items here and just have a simple counter.. that is not here, but in hte get_items function, and then exit the generator once you reach that max number.

…asses

kiddinn

quite a large PR, sorry for taking so long

laceworksdk/api/base_endpoint.py

laceworksdk/api/crud_endpoint.py

laceworksdk/api/v2/agent_access_tokens.py

laceworksdk/api/v2/alert_profiles.py

kiddinn · 2022-01-17T10:40:48Z

laceworksdk/http_session.py

-        return response
+            try:
+                next_page = response.json()['paging']['urls']['nextPage']
+            except Exception:


you marked this as resolved, yet I still see the same code, with a except Exception here

(that is to say, you did adress part of the comment, just not all of it, and not the broad exception handling aspect of it)

Ref: https://github.com/psf/requests/blob/v2.22.0/requests/packages.py

kiddinn

Two nits, otherwise LGTM

laceworksdk/__init__.py

laceworksdk/http_session.py

alannix-lw added help wanted Extra attention is needed draft enhancement New feature or request labels Dec 23, 2021

alannix-lw requested review from hazedav and kiddinn December 29, 2021 04:37

alannix-lw changed the title ~~feat: added initial function to process nextPage for pagination~~ Adding pagination functionality Dec 29, 2021

hazedav reviewed Dec 31, 2021

View reviewed changes

laceworksdk/http_session.py Outdated Show resolved Hide resolved

hazedav reviewed Dec 31, 2021

View reviewed changes

laceworksdk/http_session.py Outdated Show resolved Hide resolved

hazedav reviewed Dec 31, 2021

View reviewed changes

laceworksdk/http_session.py Outdated Show resolved Hide resolved

hazedav reviewed Dec 31, 2021

View reviewed changes

alannix-lw force-pushed the alannix-lw/pagination branch from 4b7f888 to 44a4c2f Compare January 4, 2022 05:35

kiddinn reviewed Jan 4, 2022

View reviewed changes

alannix-lw added the major label Jan 6, 2022

alannix-lw changed the title ~~Adding pagination functionality~~ Added new APIv2 endpoints that allow advanced searches and included pagination functionality Jan 12, 2022

alannix-lw force-pushed the alannix-lw/pagination branch from 8475324 to 2148ae9 Compare January 12, 2022 16:46

alannix-lw removed the draft label Jan 12, 2022

Alan Nix added 6 commits January 12, 2022 12:07

refactor: updated HttpSession class to be much more flexible

d198061

feat: allow org-level access and sub-account to be modified dynamically

8386f54

refactor: created three base class types that endpoints should inherit

b585fce

refactor: reorganized existing code and refactored to use new base cl…

1c6f698

…asses

feat: implemented all new APIv2 endpoints including pagination support

40c66b6

chore: included flake8-quotes in dev dependencies

5145e53

alannix-lw force-pushed the alannix-lw/pagination branch from 2148ae9 to 5145e53 Compare January 12, 2022 17:08

alannix-lw marked this pull request as ready for review January 12, 2022 17:08

docs: small update to README.md for latest API endpoints

b884556

alannix-lw force-pushed the alannix-lw/pagination branch from 1010920 to b884556 Compare January 15, 2022 19:36

kiddinn self-requested a review January 17, 2022 07:53

kiddinn suggested changes Jan 17, 2022

View reviewed changes

refactor: corrected docs and simplified build_dict_from_items method

f4ceeb6

Alan Nix added 20 commits January 19, 2022 13:05

fix: sanitizing access to 'query_data'

8bc018a

fix: narrowing down exception handling for pagination iteration

11f4fcc

refactor: moved resource attribute to base of SeachEndpoint

48ea0d6

refactor: removed unnecessary override method

af5f0ec

docs: added docstrings for 'query_data' where needed

b6bbde2

docs: added docstrings for methods which are passed

96b50b1

refactor: changed LaceworkException name to match conventions

2397048

tests: massively de-duped code for new search API tests

689901e

chore: importing Retry directly from urllib

3e6a62e

Ref: https://github.com/psf/requests/blob/v2.22.0/requests/packages.py

fix: passthrough Lacework response when maximum retries reached

9525938

fix: supply json to the query execute function

861fbaa

docs: improved function documentation for the BaseEndpoint class

152bee4

chore: removed references to query_data in favor of json

0817ad9

chore: removed redundant search() method overrides

784f470

fix: improved consistency of variable naming in AgentAccessTokensAPI

7d123b8

chore: improved error message for JSONDecodeError

faf4ec4

docs: added additional class docstrings

cf80f2d

fix: fixed bugs and consistency issues found in testing

384704b

refactor: simplified and modernized all APIv2 tests

7823cdc

tests: fixed dependency issue with tests

83880bd

alannix-lw requested a review from kiddinn January 28, 2022 03:50

kiddinn approved these changes Jan 31, 2022

View reviewed changes

laceworksdk/__init__.py Outdated Show resolved Hide resolved

laceworksdk/http_session.py Outdated Show resolved Hide resolved

Alan Nix added 2 commits January 31, 2022 10:08

fix: modified error logging for 'nextPage' parsing

157b036

refactor: changed LaceworksdkException to LaceworkSDKException

1624ff8

alannix-lw removed the help wanted Extra attention is needed label Jan 31, 2022

alannix-lw merged commit ee3f6f8 into master Jan 31, 2022

alannix-lw deleted the alannix-lw/pagination branch January 31, 2022 15:37

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Added new APIv2 endpoints that allow advanced searches and included pagination functionality #47

Added new APIv2 endpoints that allow advanced searches and included pagination functionality #47

alannix-lw commented Dec 23, 2021

hazedav Dec 31, 2021

alannix-lw Dec 31, 2021

hazedav Dec 31, 2021 •

edited

Loading

hazedav Dec 31, 2021

alannix-lw Dec 31, 2021

kiddinn Jan 4, 2022

kiddinn left a comment

kiddinn Jan 4, 2022

kiddinn Jan 17, 2022

kiddinn Jan 4, 2022

kiddinn left a comment

kiddinn Jan 17, 2022

kiddinn left a comment

		@@ -29,49 +32,84 @@ def __init__(self, session):
		def get(self,

Added new APIv2 endpoints that allow advanced searches and included pagination functionality #47

Added new APIv2 endpoints that allow advanced searches and included pagination functionality #47

Conversation

alannix-lw commented Dec 23, 2021

Choose a reason for hiding this comment

Choose a reason for hiding this comment

hazedav Dec 31, 2021 • 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

kiddinn left a 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

kiddinn left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

kiddinn left a comment

Choose a reason for hiding this comment

hazedav Dec 31, 2021 •

edited

Loading