feat: Add json setting to allow unicodes to show in log instead of ascii ch…

[ysde]

…aracter (for example, show chinese word '哈' in log instead of '\u8bed')

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 #<192> 🦕

[daniel-sanche]

Thanks for the contribution. I agree with the root problem (Unicode characters should be kept in the final logs), but I'm not sure if this is the best way to go about it:

• We should try to fix this in a way that applies to all environments, instead of just the Container Engine handler.
• It seems like unicode should be the default behavior, instead of an option that needs to be turned on.
• Before merging this, we'd want to make sure we put a bunch of tests in place (environment, system, and unit).

If you're interested in making those changes to this PR, that would be great. Otherwise, I can open a new issue and make sure it's addressed in a future release (this seems like it would be high priority on my priorities, so likely next release. But I'll have to do some more testing to make sure)

[ysde]

Hi @daniel-sanche

Sure I'd like to make those changes to this PR. Do you have any suggestions where or which files should I started it?
And what should I do to pass all the checks ?

Thank you very much

[daniel-sanche]

The first step would be to add a new test to the new environment test submodule to see which GCP platforms are impacted. I did that part myself here (you can take over the submodule, branch, or copy/paste the code)

It looks like only GKE is having issues there, which makes it simple (possibly cloud run too, but I think that's flakiness in the new tests - I'm looking into it). You can probably keep the json.dumps(payload, ensure_ascii=False) part of your code to fix it, but I don't think we need the other parts (I can't think of any situations where it would be useful to turn it off, unless you disagree)

After you have those fixes in and the environment test is passing, you should add similar checks to the unit and system tests.

Our test system is in the process of being upgraded, so it may be a bit confusing/flaky. Feel free to reach out or re-assign the work back to me if you get stuck with the tests!

[daniel-sanche]

Hey, any updates? If you're finding it hard to navigate the tests, I'm happy to take this over to get it in the next release. Let me know

[ysde]

Hi @daniel-sanche

Sorry for the late response, busy at working recently. :(

I'm not sure if there is some circumstances need to set ensure_ascii=True, so I was trying to keep the same behavior of the program, and think maybe add a json_ensure_ascii setting, default is true is good.

I have little experience of using submodule and not sure how to take over environment test submodule.

I've modified test_container_engine.py, but have ModuleNotFoundError: No module named 'google.cloud' when I try to execute pytest -s test_container_engine.py

Can you give me this weekend to figure out how to finish it?

If it's releasing soon, you can take it over, no worries. : )

Thank you for the help and reviews.

[daniel-sanche]

Ok, to make things simple you can ignore the environment test submodule, I can handle adding a test there after you're done.

For the unit tests, you should be able to run them through nox, which will simplify dependency issues. Information can be found in the repo's CONTRIBUTING.md file.

Let me know if you have other questions!

[ysde]

@daniel-sanche

Thanks! nox is so cool! I passed nox -s --unit-3.6

And how to make the required checks pass?

Kokoro
Samples - Lint
Samples - Python 3.6
Samples - Python 3.7
Samples - Python 3.8

Thank you

[daniel-sanche]

And how to make the required checks pass?

It looks like the CI tests are now passing. Are you still having issues?

[ysde]

And how to make the required checks pass?

It looks like the CI tests are now passing. Are you still having issues?

Nope, CI's just not triggered that time, thank you ! :)