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

Provenance script tweaks #233

Merged
merged 12 commits into from
Jan 20, 2023
Merged

Provenance script tweaks #233

merged 12 commits into from
Jan 20, 2023

Conversation

alexdunnjpl
Copy link
Contributor

πŸ—’οΈ Summary

This started out as an attempt to understand the script and figure out why it ostensibly wasn't working on my local dev instance (it was - the API version I was using was old).

Cliffnotes:

  • argument parsing is moved to the main block to allow run() to be called from other contexts
  • tweaked logging and changed default level to INFO (otherwise, a successful run yields no output, which is confusing)
  • simplified some status checks that were just throwing on non-200
  • renamed some labels for clarity
  • removed extraneous no-op statements
  • Python-consistent styling (whitespace, line-breaks). I won't say PEP because it wasn't a comprehensive lint.

I've left the commits granular in case some of these changes aren't desirable (kick them back to me and I'll fix)


- getting more help on availables arguments and what is expected:
if filepath:
handlers.append(logging.FileHandler(filepath))
Copy link

Choose a reason for hiding this comment

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

52% of developers fix this issue

Incompatible parameter type: Expected logging.StreamHandler[typing.TextIO] for 1st positional only parameter to call list.append but got logging.FileHandler.


ℹ️ Learn about @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.


Was this a good recommendation?
[ πŸ™ Not relevant ] - [ πŸ˜• Won't fix ] - [ πŸ˜‘ Not critical, will fix ] - [ πŸ™‚ Critical, will fix ] - [ 😊 Critical, fixing now ]

log.info('completed CLI processing')


def get_historic(provenance: {str: str}, reset: bool) -> {str: str}: # TODO: populate comment and rename for clarity
Copy link

Choose a reason for hiding this comment

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

19% of developers fix this issue

πŸ’¬ 3 similar findings have been found in this PR


Invalid type: Expression `{ str


πŸ”Ž Expand here to view all instances of this finding
File Path Line Number
support/provenance.py 73
support/provenance.py 99
support/provenance.py 139

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


ℹ️ Learn about @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.


Was this a good recommendation?
[ πŸ™ Not relevant ] - [ πŸ˜• Won't fix ] - [ πŸ˜‘ Not critical, will fix ] - [ πŸ™‚ Critical, will fix ] - [ 😊 Critical, fixing now ]

Copy link
Contributor

@jimmie jimmie left a comment

Choose a reason for hiding this comment

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

Nice changes.

@jimmie jimmie merged commit 1ed1711 into main Jan 20, 2023
@jimmie jimmie deleted the provenance-script-tweaks branch January 20, 2023 22:30
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants