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

Add support for python 3.10 #5379

Merged
merged 12 commits into from Aug 9, 2022
Merged

Add support for python 3.10 #5379

merged 12 commits into from Aug 9, 2022

Conversation

gherceg
Copy link
Contributor

@gherceg gherceg commented Aug 1, 2022

https://dimagi-dev.atlassian.net/browse/SAAS-13500

First step in upgrading commcare-cloud to work with python 3.10. Once tests pass, we should be able to test this on staging.

ENVIRONMENTS AFFECTED

None

@gherceg
Copy link
Contributor Author

gherceg commented Aug 8, 2022

There are two entry points in our build that depend on the generated help files from the ansible -h and ansible-playbook -h commands. As of 3.10, the output for <ansible-command> -h changed ever so slightly from "optional arguments:" to "options:". This is important because we use this syntax to key off when parsing help messages (here and here)

The first entrypoint occurs when running commcare-cloud testenv deploy-stack --skip-check --quiet --tags=py3,commcarehq. In this case, the "cached" files are used as is from the repository which at the moment means they were generated with python 3.6 and have "optional arguments:" syntax.

The second entrypoint occurs when running the test_autogen_docs.sh script, which starts by running make clean and removing all "cached" files, and then regenerating them. This means that when running with 3.10, the files contain the new "options:" syntax.

The logic to resolve the parsing is straightforward, and could either be the current implementation which is to grab the correct syntax based on the python version, or just use a regex pattern to match against that contains both strings (leaning towards changing to this).

I think the more important factor is how to handle the git diff that results from generating these files on an env with 3.10. The change to the ansible.txt and ansible-playbook.txt files on 3.10 are flagged by the fail_if_there_is_a_git_diff.sh script as not being logged properly in our changelog. This would make it tricky to support multiple versions of python currently (assuming that is desirable), so we would either:

  • remove these files from git and leave it up to specific environments to generate them
  • continue to only target one version of python at a time

@millerdev
Copy link
Contributor

Could we do something to eliminate the diffs in those files? Like post-process the ansible help output to standardize lines with expected diffs before saving it in the "cache" file?

@@ -20,7 +20,7 @@ def _get_help_text(command):
with open(_AVAILABLE_HELP_CACHES[command], 'r', encoding='utf-8') as f:
return f.read()
else:
return subprocess.check_output(command, shell=True)
return subprocess.check_output(command, shell=True).decode('utf-8')
Copy link
Contributor Author

Choose a reason for hiding this comment

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

I added this when playing around with removing the cache entirely as I noticed this returned a byte string previously. At the moment it looks like we never actually invoke this code since only the two ansible commands use this method, and definitely hit the cache. I think this is a safe change, but wanted to flag since it isn't directly related to any 3.10 compatibility.

@@ -32,7 +32,7 @@
]
test_deps = [
'modernize',
'nose>=1.3.7',
'nose @ git+https://github.com/dimagi/nose.git@06dff28bbe661b9d032ce839ea0ec8e9eaf6f337',
Copy link
Member

Choose a reason for hiding this comment

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

Looks like nose hasn't received an update since 2015. Is it time to remove it from our test stack here? (Not blocking this PR.)

Copy link
Contributor

Choose a reason for hiding this comment

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

This is a bigger topic, so I agree that it should not block this PR. My vote is to keep nose until pytest has a solution for pytest-dev/pytest#3834

Makefile Outdated Show resolved Hide resolved
@@ -32,7 +32,7 @@
]
test_deps = [
'modernize',
'nose>=1.3.7',
'nose @ git+https://github.com/dimagi/nose.git@06dff28bbe661b9d032ce839ea0ec8e9eaf6f337',
Copy link
Contributor

Choose a reason for hiding this comment

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

This is a bigger topic, so I agree that it should not block this PR. My vote is to keep nose until pytest has a solution for pytest-dev/pytest#3834

@gherceg gherceg merged commit 10884e5 into master Aug 9, 2022
@gherceg gherceg deleted the gh/support-3.10 branch August 9, 2022 21:20
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.

None yet

3 participants