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

qa: set locale to C.UTF-8 in tox.ini #49321

Merged
merged 4 commits into from Dec 9, 2022

Conversation

tchaikov
Copy link
Contributor

@tchaikov tchaikov commented Dec 8, 2022

as ansible is using UTF-8 encoded characters in the file names, so, to avoid failures like:

File "/home/jenkins-build/build/workspace/ceph-pull-requests/qa/.tox/py3/lib/python3.10/site-packages/pip/_internal/utils/unpacking.py", line 217, in untar_file
with open(path, "wb") as destfp:
UnicodeEncodeError: 'latin-1' codec can't encode characters in position 137-140: ordinal not in range(256)

we have to set a locale which is able to handle UTF-8.

see also ceph/teuthology#1671

Signed-off-by: Kefu Chai tchaikov@gmail.com

Contribution Guidelines

Checklist

  • Tracker (select at least one)
    • References tracker ticket
    • Very recent bug; references commit where it was introduced
    • New feature (ticket optional)
    • Doc update (no ticket needed)
    • Code cleanup (no ticket needed)
  • Component impact
    • Affects Dashboard, opened tracker ticket
    • Affects Orchestrator, opened tracker ticket
    • No impact that needs to be tracked
  • Documentation (select at least one)
    • Updates relevant documentation
    • No doc update is appropriate
  • Tests (select at least one)
Show available Jenkins commands
  • jenkins retest this please
  • jenkins test classic perf
  • jenkins test crimson perf
  • jenkins test signed
  • jenkins test make check
  • jenkins test make check arm64
  • jenkins test submodules
  • jenkins test dashboard
  • jenkins test dashboard cephadm
  • jenkins test api
  • jenkins test docs
  • jenkins render docs
  • jenkins test ceph-volume all
  • jenkins test ceph-volume tox
  • jenkins test windows

@tchaikov tchaikov changed the title qa: set local to en_US.UTF-8 in tox.ini qa: set locale to en_US.UTF-8 in tox.ini Dec 8, 2022
@tchaikov
Copy link
Contributor Author

tchaikov commented Dec 8, 2022

this change should address the first failure in https://jenkins.ceph.com/job/ceph-pull-requests/107859/console

@tchaikov
Copy link
Contributor Author

tchaikov commented Dec 8, 2022

@adk3798 hi Adam, could you help review this change as well?

@adk3798
Copy link
Contributor

adk3798 commented Dec 8, 2022

still seeing

usage: mypy [-h] [-v] [-V] [more options; see below]
            [-m MODULE] [-p PACKAGE] [-c PROGRAM_TEXT] [files ...]
mypy: error: Missing target module, package, files, or command.
mypy: exit 2 (0.22 seconds) /home/jenkins-build/build/workspace/ceph-pull-requests/src/pybind/mgr> mypy --config-file=../../mypy.ini pid=3198086
mypy: FAIL ✖ in 35.1 seconds

but thought this looked alright.

@tchaikov
Copy link
Contributor Author

tchaikov commented Dec 8, 2022

that's a different issue from the one fixed by this change. i think this is also a fallout of upgrading tox -- it could be interpreting the newlines in a variable differently.

@adk3798
Copy link
Contributor

adk3798 commented Dec 8, 2022

that's a different issue from the one fixed by this change. i think this is also a fallout of upgrading tox -- it could be interpreting the newlines in a variable differently.

okay, well this PR looks good then anyway

Copy link
Contributor

@phlogistonjohn phlogistonjohn left a comment

Choose a reason for hiding this comment

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

Hi @tchaikov can we use the C.UTF-8 locale instead? As part of my efforts to build and test in containers, many of the images only have a minimal set of locales included. For example I had to change install-deps.sh to prevent errors when running in the container (see cd1cd1b)

Thanks!

@phlogistonjohn
Copy link
Contributor

I'd also mention that I think my build-and-test-in-containers effort could help avoid these sudden scrambles to fix things when packages on the build hosts change ;-)
If I understand the cause of the sudden need to update our tox.ini files correctly.

as ansible is using UTF-8 encoded characters in the file names, so,
to avoid failures like:

  File "/home/jenkins-build/build/workspace/ceph-pull-requests/qa/.tox/py3/lib/python3.10/site-packages/pip/_internal/utils/unpacking.py", line 217, in untar_file
    with open(path, "wb") as destfp:
UnicodeEncodeError: 'latin-1' codec can't encode characters in position 137-140: ordinal not in range(256)

we have to set a locale which is able to handle UTF-8.

see also ceph/teuthology#1671

Signed-off-by: Kefu Chai <tchaikov@gmail.com>
@tchaikov
Copy link
Contributor Author

tchaikov commented Dec 8, 2022

Hi @tchaikov can we use the C.UTF-8 locale instead? As part of my efforts to build and test in containers, many of the images only have a minimal set of locales included. For example I had to change install-deps.sh to prevent errors when running in the container (see cd1cd1b)

Thanks!

@phlogistonjohn hi John, done.

@tchaikov
Copy link
Contributor Author

tchaikov commented Dec 8, 2022

I'd also mention that I think my build-and-test-in-containers effort could help avoid these sudden scrambles to fix things when packages on the build hosts change ;-)
If I understand the cause of the sudden need to update our tox.ini files correctly.

looking forward to it!

Copy link
Contributor

@phlogistonjohn phlogistonjohn left a comment

Choose a reason for hiding this comment

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

LGTM

@tchaikov tchaikov changed the title qa: set locale to en_US.UTF-8 in tox.ini qa: set locale to C.UTF-8 in tox.ini Dec 8, 2022
otherwise it is mising when running test, and we'd have following
failure:

py3: exit 2 (0.00 seconds) /home/jenkins-build/build/workspace/ceph-pull-requests/qa> pytest --assert=plain test_import.py

Signed-off-by: Kefu Chai <tchaikov@gmail.com>
@tchaikov
Copy link
Contributor Author

tchaikov commented Dec 8, 2022

jenkins test make check

py3 tries to import all python modules to ensure that they are
python3 compatible. but the installation fails on jenkins test node:

  Resolved https://github.com/ceph/teuthology.git to commit 4da97cf64e542f347ec47b7bdbe5eca99759f9b7
  Installing build dependencies: started
  error: subprocess-exited-with-error

Signed-off-by: Kefu Chai <tchaikov@gmail.com>
as we always test with ubuntu jammy, which does not provide python3.7:

py37: skipped because could not find python interpreter with spec(s): py37

so there is no point testing with python3.7.

also, in tox v4, it is not able to handle "key = value" anymore, where
value has newlines in it. so we need to find a better way passing
command line options to the test command.

this change partially reverts 2dd86c9

Signed-off-by: Kefu Chai <tchaikov@gmail.com>
@tchaikov tchaikov merged commit af7d7d3 into ceph:main Dec 9, 2022
@tchaikov tchaikov deleted the wip-qa-install-with-utf-8 branch December 9, 2022 00:31
adk3798 added a commit to adk3798/ceph that referenced this pull request Dec 10, 2022
When creating ceph#49359 I was testing on an outdated
branch and didn't realize part of what I was fixing
had already been fixed in ceph#49321. Basically ended up
changing what a variable "mypy_args" is set to but that
variable is no longer being used. It has no actual effect
but we should remove the extraneous code.

Signed-off-by: Adam King <adking@redhat.com>
mgfritch pushed a commit to mgfritch/ceph that referenced this pull request Dec 12, 2022
When creating ceph#49359 I was testing on an outdated
branch and didn't realize part of what I was fixing
had already been fixed in ceph#49321. Basically ended up
changing what a variable "mypy_args" is set to but that
variable is no longer being used. It has no actual effect
but we should remove the extraneous code.

Signed-off-by: Adam King <adking@redhat.com>
aaSharma14 pushed a commit to rhcs-dashboard/ceph that referenced this pull request Feb 15, 2023
When creating ceph#49359 I was testing on an outdated
branch and didn't realize part of what I was fixing
had already been fixed in ceph#49321. Basically ended up
changing what a variable "mypy_args" is set to but that
variable is no longer being used. It has no actual effect
but we should remove the extraneous code.

Signed-off-by: Adam King <adking@redhat.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
3 participants