feat: Allow filtering for EnvironmentParser #447

tmeckel · 2022-11-30T11:47:22Z

This PR contains changes to the EnvironmentParser to allow the filtering of packages read from pkg_resources.working_set.

The PR overlaps with #444

As requested from CONTRIBUTING.md:

commits are signed
documentation added
formatted with isort and autopep8
added new tests for implemented functionality
run tests locally successful with poetry run tox

* added optional parameter location_filter to __init__ to allow filtering of packages by their installation location * added optional parameter requirements_content to __init__ to allow filtering of packages by their package names read from a requirements file * added filter expression for location_filter and requirements_content Signed-off-by: Thomas Meckel <tmeckel@users.noreply.github.com>

* added test_simple_use_location_filter * added test_simple_use_requiremets Signed-off-by: Thomas Meckel <tmeckel@users.noreply.github.com>

* added argument --location-filter to input_method_group * changed initialization for environment parser based on new filter arguments Signed-off-by: Thomas Meckel <tmeckel@users.noreply.github.com>

* added missing single quotes to remove WARNING: Could not lex literal_block as "bash" * added documentation for new filter arguments for environment parser Signed-off-by: Thomas Meckel <tmeckel@users.noreply.github.com>

tests/test_parser_environment.py

jkowalleck · 2022-12-01T08:43:04Z

docs/usage.rst

                            Use with -i to specify absolute path to a
-                            `requirements.txt` you wish to use, else we'll look
+                            `requirements.txt` you wish to use, else we''ll look


note for myself -> need to check if this is correctly rendered to RTD

jkowalleck · 2022-12-01T08:45:58Z

cyclonedx_py/client.py

@@ -206,6 +206,13 @@ def get_arg_parser(*, prog: Optional[str] = None) -> argparse.ArgumentParser:
            default=None,
            help='File to read input from. Use "-" to read from STDIN.', dest='input_source', required=False
        )
+        input_method_group.add_argument(
+            '--location-filter', action='store',
+            default=None, nargs='+',


note to self: double-check some edge-cases

jkowalleck · 2022-12-01T08:47:37Z

@tmeckel could you add the new CLI option to the https://github.com/CycloneDX/cyclonedx-python#basic-usage ?

tmeckel · 2022-12-01T09:07:57Z

@jkowalleck ~~I updated the usage.rst in starting at lines 34 and 62. Do you want me to add some more documentation?~~
Sorry, it's the README.mdmy oversight. I'll add this today.

tmeckel · 2022-12-01T09:12:20Z

@jkowalleck will try to fix the failing test on Windows test_parser_environment.py:72 as well.

Traceback (most recent call last):
  File "D:\a\cyclonedx-python\cyclonedx-python\tests\test_parser_environment.py", line 72, in test_simple_use_location_filter
    self.assertGreater(parser_sys_path.component_count(), 1)
AssertionError: 0 not greater than 1

… README.md Signed-off-by: Thomas Meckel <tmeckel@users.noreply.github.com>

Signed-off-by: Thomas Meckel <tmeckel@users.noreply.github.com>

…r location_filter to lowercase to match location properties from pkg_resources.working_set Signed-off-by: Thomas Meckel <tmeckel@users.noreply.github.com>

tmeckel · 2022-12-01T11:02:31Z

@jkowalleck I pushed changes to add the requested additions to basic usage in README.md and fixed (hopefully) the failing test on Windows.

tmeckel · 2022-12-01T15:11:34Z

cyclonedx_py/parser/environment.py

+
+            rf_names = set([req.name for req in parsed_rf.requirements])
+
+        if os.name == 'nt' and location_filter:


@jkowalleck had to add this code to get the location filter running on Windows. sys.path elements are Camel-Case on Windows. Location properties from pkg_resources.working_set are all lower case.

note to self: need to double-check on that

a1lu · 2022-12-21T20:36:58Z

Any progress on this? I would like to see this feature.

jkowalleck · 2022-12-22T00:08:23Z

To move this forward, some git conflicts need to be sorted out first.
@tmeckel, could you look into this?

jkowalleck · 2022-12-22T08:22:05Z

cyclonedx_py/client.py

+            input_data_fh = self._arguments.input_source
+            with input_data_fh:
+                input_data = input_data_fh.read()
+                input_data_fh.close()


❌
input_data_fh.close() not needed, as it will happen automatically on exit of the with block

jkowalleck · 2022-12-22T08:23:59Z

cyclonedx_py/client.py

@@ -251,7 +258,14 @@ def _error_and_exit(message: str, exit_code: int = 1) -> None:

    def _get_input_parser(self) -> BaseParser:
        if self._arguments.input_from_environment:
-            return EnvironmentParser(use_purl_bom_ref=self._arguments.use_purl_bom_ref)
+            input_data_fh = self._arguments.input_source
+            with input_data_fh:


what if input_source is None? will the with block crash?

jkowalleck · 2022-12-22T09:59:37Z

@tmeckel @a1lu before putting more effort in this PR,
please read #435 (comment)

You will find good reasons why I will probably not merge this PR.
I see your solution working for your current use cases, but it appears to me not wholesome enough.
Therefore, i (temporary) closed this PR.

tmeckel · 2022-12-22T10:27:48Z

@jkowalleck I'm totally fine with your decision because in the meantime I fixed the issue that lead to this PR using a different approach.

jkowalleck · 2022-12-22T10:47:36Z

@tmeckel re: #447 (comment)
I am happy to hear.

Would you be so kind and open a new Q&A in https://github.com/CycloneDX/cyclonedx-python/discussions/categories/q-a
where you describe the problem and answer your own "Question" with the solution you found?

tmeckel requested a review from a team as a code owner November 30, 2022 11:47

tmeckel added 4 commits November 30, 2022 11:48

Changes to tests/test_parser_environment.py:

6a1ee6a

* added test_simple_use_location_filter * added test_simple_use_requiremets Signed-off-by: Thomas Meckel <tmeckel@users.noreply.github.com>

Changes to cyclonedx_py/client.py:

f8bc4c7

* added argument --location-filter to input_method_group * changed initialization for environment parser based on new filter arguments Signed-off-by: Thomas Meckel <tmeckel@users.noreply.github.com>

Changes to docs/usage.rst:

6140784

* added missing single quotes to remove WARNING: Could not lex literal_block as "bash" * added documentation for new filter arguments for environment parser Signed-off-by: Thomas Meckel <tmeckel@users.noreply.github.com>

sonatype-lift bot reviewed Nov 30, 2022

View reviewed changes

tests/test_parser_environment.py Show resolved Hide resolved

sonatype-lift bot reviewed Nov 30, 2022

View reviewed changes

tests/test_parser_environment.py Show resolved Hide resolved

jkowalleck reviewed Dec 1, 2022

View reviewed changes

tmeckel added 3 commits December 1, 2022 09:52

Added documentation about new filter options for EnvironmentParser to…

29adcb5

… README.md Signed-off-by: Thomas Meckel <tmeckel@users.noreply.github.com>

Added missing [--location-filter] to usage line in docs/usage.rst

141953d

Signed-off-by: Thomas Meckel <tmeckel@users.noreply.github.com>

On Windows EnvironmentParser will convert paths contained in paramete…

b995932

…r location_filter to lowercase to match location properties from pkg_resources.working_set Signed-off-by: Thomas Meckel <tmeckel@users.noreply.github.com>

tmeckel requested a review from jkowalleck December 1, 2022 11:05

tmeckel commented Dec 1, 2022

View reviewed changes

Merge branch 'master' into feature/environment-filters

7e1dc28

jkowalleck requested changes Dec 22, 2022

View reviewed changes

jkowalleck closed this Dec 22, 2022

tmeckel deleted the feature/environment-filters branch December 22, 2022 10:27

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat: Allow filtering for EnvironmentParser #447

feat: Allow filtering for EnvironmentParser #447

tmeckel commented Nov 30, 2022 •

edited

Loading

jkowalleck Dec 1, 2022

jkowalleck Dec 1, 2022

jkowalleck commented Dec 1, 2022

tmeckel commented Dec 1, 2022 •

edited

Loading

tmeckel commented Dec 1, 2022

tmeckel commented Dec 1, 2022

tmeckel Dec 1, 2022

jkowalleck Dec 1, 2022 •

edited

Loading

a1lu commented Dec 21, 2022

jkowalleck commented Dec 22, 2022

jkowalleck Dec 22, 2022

jkowalleck Dec 22, 2022

jkowalleck commented Dec 22, 2022 •

edited

Loading

tmeckel commented Dec 22, 2022

jkowalleck commented Dec 22, 2022


		rf_names = set([req.name for req in parsed_rf.requirements])

		if os.name == 'nt' and location_filter:

feat: Allow filtering for EnvironmentParser #447

feat: Allow filtering for EnvironmentParser #447

Conversation

tmeckel commented Nov 30, 2022 • edited Loading

jkowalleck Dec 1, 2022

Choose a reason for hiding this comment

jkowalleck Dec 1, 2022

Choose a reason for hiding this comment

jkowalleck commented Dec 1, 2022

tmeckel commented Dec 1, 2022 • edited Loading

tmeckel commented Dec 1, 2022

tmeckel commented Dec 1, 2022

tmeckel Dec 1, 2022

Choose a reason for hiding this comment

jkowalleck Dec 1, 2022 • edited Loading

Choose a reason for hiding this comment

a1lu commented Dec 21, 2022

jkowalleck commented Dec 22, 2022

jkowalleck Dec 22, 2022

Choose a reason for hiding this comment

jkowalleck Dec 22, 2022

Choose a reason for hiding this comment

jkowalleck commented Dec 22, 2022 • edited Loading

tmeckel commented Dec 22, 2022

jkowalleck commented Dec 22, 2022

tmeckel commented Nov 30, 2022 •

edited

Loading

tmeckel commented Dec 1, 2022 •

edited

Loading

jkowalleck Dec 1, 2022 •

edited

Loading

jkowalleck commented Dec 22, 2022 •

edited

Loading