search

google / slo-generator

SLO Generator computes SLIs, SLOs, Error Budgets and Burn Rates from supported backends, then exports an SLO report to supported targets.
Apache License 2.0
489 stars 78 forks source link

fix(lint): pytype checks #321

Closed smehboub closed 1 year ago

smehboub commented 1 year ago

fix(lint): pytype checks

lvaylet commented 1 year ago

Hi @smehboub, thanks for submitting this PR. Could you elaborate a bit on its purpose and what is it supposed to fix?

lvaylet commented 1 year ago

Got it. I ran into issues while working on another PR. Here is what I did:

  1. I created a bug to report the behavior: https://github.com/google/slo-generator/issues/322
  2. I wrote a dedicated PR with meaningful commit messages: https://github.com/google/slo-generator/pull/323

Regarding the pytype fix, I decided to configure the module name in pyproject.toml so it is automatically detected by either make lint or pytype.

Regarding the safety check fix, hiding or ignoring warnings is rarely the best option. I decided to fix the issue by pinning the version of setuptools.

Does that make sense? If so, feel free to rebase your other PR to catch up with the latest changes.

smehboub commented 1 year ago

Got it. I ran into issues while working on another PR. Here is what I did:

  1. I created a bug to report the behavior: bug [BUG] - make lint fails with the latest version of pytype #322
  2. I wrote a dedicated PR with meaningful commit messages: ci: make lint fails with latest version of pytype and vulnerable setuptools #323

Regarding the pytype fix, I decided to configure the module name in pyproject.toml so it is automatically detected by either make lint or pytype.

Regarding the safety check fix, hiding or ignoring warnings is rarely the best option. I decided to fix the issue by pinning the version of setuptools.

Does that make sense? If so, feel free to rebase your other PR to catch up with the latest changes.

That's most clean, thanks. I would have liked to do it together, to be a contributor to the slo-generator ;-)

lvaylet commented 1 year ago

@smehboub You will have plenty of other opportunities :-) Contributors are most welcome!