Skip to content
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

feat: metadata.component for poetry #542

Closed
wants to merge 2 commits into from

Conversation

italvi
Copy link

@italvi italvi commented Apr 21, 2023

This PR tries to address #391

Currently I only check for a pyproject.toml. As this will be used in combination with poetry I only added this feature there.

@italvi italvi requested a review from a team as a code owner April 21, 2023 10:01
@@ -111,7 +111,7 @@ def get_output(self) -> BaseOutput:
'',
sep='\n', file=sys.stderr)

bom = Bom(components=filter(self._component_filter, parser.get_components()))
bom = Bom(components=filter(self._component_filter, parser.get_components()), metadata=parser._metadata)
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

E1123: Unexpected keyword argument 'metadata' in constructor call


ℹ️ Expand to see all @sonatype-lift commands

You can reply with the following commands. For example, reply with @sonatype-lift ignoreall to leave out all findings.

Command Usage
@sonatype-lift ignore Leave out the above finding from this PR
@sonatype-lift ignoreall Leave out all the existing findings from this PR
@sonatype-lift exclude <file|issue|path|tool> Exclude specified file|issue|path|tool from Lift findings by updating your config.toml file

Note: When talking to LiftBot, you need to refresh the page to see its response.
Click here to add LiftBot to another repo.

@@ -111,7 +111,7 @@ def get_output(self) -> BaseOutput:
'',
sep='\n', file=sys.stderr)

bom = Bom(components=filter(self._component_filter, parser.get_components()))
bom = Bom(components=filter(self._component_filter, parser.get_components()), metadata=parser._metadata)
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

12% of developers fix this issue

reportGeneralTypeIssues: No parameter named "metadata"

❗❗ 2 similar findings have been found in this PR

🔎 Expand here to view all instances of this finding
File Path Line Number
cyclonedx_py/client.py 114
cyclonedx_py/parser/poetry.py 104

Visit the Lift Web Console to find more details in your report.


ℹ️ Expand to see all @sonatype-lift commands

You can reply with the following commands. For example, reply with @sonatype-lift ignoreall to leave out all findings.

Command Usage
@sonatype-lift ignore Leave out the above finding from this PR
@sonatype-lift ignoreall Leave out all the existing findings from this PR
@sonatype-lift exclude <file|issue|path|tool> Exclude specified file|issue|path|tool from Lift findings by updating your config.toml file

Note: When talking to LiftBot, you need to refresh the page to see its response.
Click here to add LiftBot to another repo.

parser = PoetryFileParser(poetry_lock_filename=poetry_lock_filename,
use_purl_bom_ref=True,
poetry_toml_filename=poetry_toml_filename)
self.assertEqual('cyclonedx-bom', parser._metadata.component.name)
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

3% of developers fix this issue

reportOptionalMemberAccess: "name" is not a known member of "None"


ℹ️ Expand to see all @sonatype-lift commands

You can reply with the following commands. For example, reply with @sonatype-lift ignoreall to leave out all findings.

Command Usage
@sonatype-lift ignore Leave out the above finding from this PR
@sonatype-lift ignoreall Leave out all the existing findings from this PR
@sonatype-lift exclude <file|issue|path|tool> Exclude specified file|issue|path|tool from Lift findings by updating your config.toml file

Note: When talking to LiftBot, you need to refresh the page to see its response.
Click here to add LiftBot to another repo.

@@ -111,7 +111,7 @@ def get_output(self) -> BaseOutput:
'',
sep='\n', file=sys.stderr)

bom = Bom(components=filter(self._component_filter, parser.get_components()))
bom = Bom(components=filter(self._component_filter, parser.get_components()), metadata=parser._metadata)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

❌ do not access protected/private properties!
use the public getters/properties!

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The current implementation of the BaseParser does not provide a get_metadata. Hence the direct access to the properties

Copy link
Member

@jkowalleck jkowalleck Apr 21, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copy link
Author

@italvi italvi Apr 21, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

would you prefer to extend it in the python-lib or shall I write a ExtendedBaseParser in this project?

I would prefer the 2nd option as the first version would require to use the most current version of the python-lib then

@@ -97,19 +99,29 @@ def __init__(
del error
self._components.append(component)

if pyproject_toml_contents:
poetry_toml = load_toml(pyproject_toml_contents)
poetry_toml_metadata = poetry_toml.get('tool', '').get('poetry', '')
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

wrong types for defaults

-             poetry_toml_metadata = poetry_toml.get('tool', '').get('poetry', '')
+             poetry_toml_metadata = poetry_toml.get('tool', {}).get('poetry', '')

@jkowalleck jkowalleck added source: poetry enhancement New feature or request labels Apr 21, 2023
@jkowalleck jkowalleck changed the title Init commit for adding metadata.component feat: metadata.component for poetry Apr 21, 2023
@jkowalleck
Copy link
Member

jkowalleck commented Apr 21, 2023

Hello @italvi ,
thank you for the contribution.

This project ins currently transitioning to v4. This means: the current develop-branch is dev/4.x.x

Could you develop your feature from there and pullrequest it to that branch?

@jkowalleck jkowalleck marked this pull request as draft April 21, 2023 10:17
@jkowalleck
Copy link
Member

converted this PR to "draft" as the target branch and source branch are not the desired ones.
see #542 (comment)

@italvi
Copy link
Author

italvi commented Apr 21, 2023

Hello @italvi , thank you for the contribution.

This project ins currently transitioning to v4. This means: the current develop-branch is dev/4.x.x

Could you develop your feature from there and pullrequest it to that branch?

@jkowalleck could you please further elaborate? I forked the project and then made my changes on the branch dev/4.x.x there

@jkowalleck
Copy link
Member

superseded by #605

@jkowalleck jkowalleck closed this Dec 25, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request source: poetry
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants