[Python 3.9 Upgrade] OpenSearch-Build Repo Python Migration to 3.9 Version

[peterzhuamazon]

Description

[Python 3.9 Upgrade] OpenSearch-Build Repo Python Migration to 3.9 Version

Issues Resolved

#3351

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.
For more information on following Developer Certificate of Origin and signing off your commits, please check here.

[codecov]

Codecov Report

Merging #3658 (608d90b) into main (ef72b1b) will increase coverage by 0.00%.
The diff coverage is n/a.

@@           Coverage Diff           @@
##             main    #3658   +/-   ##
=======================================
  Coverage   91.54%   91.54%           
=======================================
  Files         182      182           
  Lines        5419     5420    +1     
=======================================
+ Hits         4961     4962    +1     
  Misses        458      458           

see 1 file with indirect coverage changes

[peterzhuamazon]

Seems like windows is breaking with this in python-test with 3.9.13:

__________________________ TestRunAssemble.test_main __________________________

self = <tests.test_run_assemble.TestRunAssemble testMethod=test_main>
mock_recorder = <MagicMock name='BundleRecorder' id='1721159340672'>
mock_bundles = <MagicMock name='create' id='1721159748864'>
getcwd = <MagicMock name='getcwd' id='1721159757056'>
makeDirs = <MagicMock name='makedirs' id='1721159773248'>

    @patch("os.makedirs")
    @patch("os.getcwd", return_value="curdir")
    @patch("argparse._sys.argv", ["run_assemble.py", BUILD_MANIFEST])
    @patch("run_assemble.Bundles.create")
    @patch("run_assemble.BundleRecorder", return_value=MagicMock())
    def test_main(self, mock_recorder: Mock, mock_bundles: Mock, getcwd: Mock, makeDirs: Mock) -> None:
        mock_bundle = MagicMock(min_dist=MagicMock(archive_path="path"))
        mock_bundles.return_value.__enter__.return_value = mock_bundle
    
        main()
    
        mock_bundle.install_min.assert_called()
        mock_bundle.install_components.assert_called()
    
        mock_bundle.package.assert_called_with(os.path.join("curdir", "tar", "dist", "opensearch"))
    
        mock_recorder.return_value.write_manifest.assert_has_calls([
            call("path"),
            call(os.path.join("curdir", "tar", "dist", "opensearch"))
        ])  # manifest included in package
    
>       self.assertEqual(getcwd.call_count, 2)
E       AssertionError: 3 != 2

tests\test_run_assemble.py:54: AssertionError

[peterzhuamazon]

Windows can change to 3.9.7 with scoop here, no need to go to 3.9.13, tho need some checks on why this is failing tho:
https://github.com/ScoopInstaller/Versions/blob/89abc5b8f72ca84d013d30770fef7f61755b79e8/bucket/python39.json

[peterzhuamazon]

With Windows able to run Python3.9 now will debug more before merging this.

[gaiksaya]

Should we not measure the call count?

[peterzhuamazon]

No we cant. Due to some changes between 3.7 and 3.9, the number of calls on Windows specifically is mistakenly reported as actual calls + 1.

I am not able to find out exactly where this actual call is coming from or find any changes related to this in os package. Some more observation suggest even without any getcwd calls, the counter is still having a +1, which is happening even on powershell.

As of now since this is not a harmful call and the actual call is not +1, I will just pass this as called. Thanks.

[gaiksaya]

Will this lead to process being left in running state if termination is not successful? Maybe create an issue for this too to address it later?

[peterzhuamazon]

This is not related to the actual termination where a wait will happen until proper termination.
The test is trying to close a read call, that is always failing on 3.7 consistently, thus always None.

In 3.9 on some OS like ubuntu github actions runner it is still observe as None, while on macos and CentOS7 I have observed 1 which is correctly termination of the process. This is so inconsistent that even adding a 10seconds timer would not resolve this.

We probably will revisit this but this is more like a flaky test than a problematic implementation on the actual code. It is just it always fail on 3.7 but sometimes run correctly on 3.9. Thanks.

[gaiksaya]

Thanks maybe open a bug? We can take help from community too.

[prudhvigodithi]

Approved to proceed with the upgrade window. Thanks