Skip to content

Conversation

@ymotongpoo
Copy link
Contributor

Thank you for opening a Pull Request! Before submitting your PR, there are a few things you can do to make sure it goes smoothly:

  • Make sure to open an issue as a bug/issue before writing your code! That way we can discuss the change, evaluate designs, and agree on the general idea
  • Ensure the tests and linter pass
  • Code coverage does not decrease (if any source code was changed)
  • Appropriate docs were updated (if necessary)

Fixes #743

@ymotongpoo ymotongpoo requested review from a team as code owners April 13, 2023 07:22
@product-auto-label product-auto-label bot added size: xs Pull request size is extra small. api: logging Issues related to the googleapis/python-logging API. labels Apr 13, 2023
@ymotongpoo
Copy link
Contributor Author

the reason why I skipped isupper() check is anyway it requires upper() in the case of lower/mixed case. Though it may cause some extra overhead for PyMem_Malloc, it should not that large so I simplified the process.

@daniel-sanche
Copy link
Contributor

This looks great! Would you mind adding a test for this too though, since the bug must have slipped through with the existing tests?

@product-auto-label product-auto-label bot added size: m Pull request size is medium. and removed size: xs Pull request size is extra small. labels Apr 17, 2023
@ymotongpoo
Copy link
Contributor Author

@daniel-sanche I added a small unit test to check if Batch#commit() can properly convert severity strings.

Copy link
Contributor

@daniel-sanche daniel-sanche left a comment

Choose a reason for hiding this comment

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

LGTM, thanks!

@daniel-sanche daniel-sanche added the kokoro:run Add this label to force Kokoro to re-run the tests. label Apr 19, 2023
@yoshi-kokoro yoshi-kokoro removed kokoro:run Add this label to force Kokoro to re-run the tests. labels Apr 19, 2023
@daniel-sanche
Copy link
Contributor

You can ignore the environment test failures. It looks like I need to upgrade the pandas version there, I'll address that in a separate PR

It looks like there's a lint issue on the Kokoro check before this can be merged though

@ymotongpoo
Copy link
Contributor Author

I just ran nox and the result was the following (my local doesn't have Python 3.7, so unit-3.7 didn't run):

nox > Session docs was successful.
nox > Ran multiple sessions:
nox > * unit-3.7: failed
nox > * unit-3.8: success
nox > * unit-3.9: success
nox > * unit-3.10: success
nox > * unit-3.11: success
nox > * system-3.8: success
nox > * cover: success
nox > * lint: success
nox > * lint_setup_py: success
nox > * blacken: success
nox > * docs: success

I hope this fix works.

@daniel-sanche daniel-sanche added the kokoro:run Add this label to force Kokoro to re-run the tests. label Apr 20, 2023
@yoshi-kokoro yoshi-kokoro removed the kokoro:run Add this label to force Kokoro to re-run the tests. label Apr 20, 2023
@daniel-sanche daniel-sanche merged commit c1c8ce1 into googleapis:main Apr 21, 2023
@daniel-sanche
Copy link
Contributor

looks great, thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

api: logging Issues related to the googleapis/python-logging API. size: m Pull request size is medium.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Convert severity string to uppercase in Batch#log_struct

4 participants