Disabling Python 3.6 and Fixing 3.9 Unit Tests in CI Workflow#459
Disabling Python 3.6 and Fixing 3.9 Unit Tests in CI Workflow#459mambelli merged 3 commits intoHEPCloud:masterfrom
Conversation
Codecov ReportPatch coverage:
Additional details and impacted files@@ Coverage Diff @@
## master #459 +/- ##
==========================================
+ Coverage 47.20% 47.36% +0.16%
==========================================
Files 54 54
Lines 2896 2903 +7
Branches 523 497 -26
==========================================
+ Hits 1367 1375 +8
+ Misses 1428 1427 -1
Partials 101 101
Flags with carried forward coverage won't be shown. Click here to find out more.
☔ View full report in Codecov by Sentry. |
a5dd4d2 to
4a59acc
Compare
d77a98f to
b4c2122
Compare
src/decisionengine_modules/glideinwms/publishers/decisionenginemonitor.py
Outdated
Show resolved
Hide resolved
|
I requested a couple of changes to complete the migration away of Python 3.6 and I'm opening an issue in decisionengine. |
|
@mambelli, after the above requested changes were made, Python 3.9 unit tests are running successfully, however, there are some warnings being thrown. There are four warnings reported, of which three of them are Here's what I understand:
Please confirm on which of the above reported warnings needs immediate attention/addressing. |
Send this to the condor team. It is a very easy fix. Maybe an issue on github or check on their site about how to submit bugs. |
This one is not urgent but needs to be fixed. You can quickly check how we use imp and if there is a one-to-one replacement in importlib. If yes, do the replacement. If not open an issue |
|
Issue 1 I think is external. 2 may be something we add in the configuration of the unit tests. Maybe the timeout option was supported and is no more or we specify it wrongly? Please check. |
UPDATE (post PR review)
warning#1 has been ignored as suggested since
Warning#3, as suggested, was reported to the HTCondor team and Cole Bollig, from their team, informed that the |
- disabled the workflow to remove the unit tests completely for python 3.6 upon confirmation that python 3.6 is no longer supported on Ubuntu 22.04 (used in the CI workflow) - updated the requirement for the version of pandas to be at least 1.5.3 when python installed is >=3.7 because 'pandas.core.computation.ops.UndefinedVariableError` has been exposed via `pandas.errors` module starting pandas 1.5.0 - changed the groupby grouper to use the string directly -- pandas version >= 1.5.0 throws FutureWarning that a length 1 tuple will be returned when iterating over the values returned with a grouper equal to a list of length 1. -- when a single grouper is used, the grouper can be specified without the list but just the string, hence avoiding the warning - changed the deprecated functionality for append method -- replaced append method with the concat method as append functionality was deprecated since pandas 1.4.0 and completely removed in 2.0.0.
ab47dfc to
fa40116
Compare
- updated to running unit tests for 3.8 instead of the initial suggestion to switch to 3.7 as 3.7 will soon be reaching EOL (June 2023). - commented the timeout option in the toml file to avoid the DeprecationWarning associated with it during the unit tests on python 3.9 - changed the glideinwms ref from `branch_v3_9` to `master` in the yaml file for the CI as the latter is the running branch for GWMS v3.9.x
Regarding warning#4, upon going through a couple of SO discussions, it seemed like this would be a 1:1 replacement (meaning just changing all references of From the documentation for find_module and load_module, these two methods have been deprecated since Python 3.3. Unless compatibility with Python 3.3 is desired, these would need to be replaced with Since **NOTE: **This has also been documented under the GlideinWMS project issue#300. Despite the fix comes in from issue#300, this would still lead to the cascading DeprecationWarning in |
As part of completing my activity on the PR for making CONTINUE_IF_NO_PROXY a configurable attribute (#457), an attempt was made to push the changes to the respective branch on the decisionengine codebase. The CI workflow reported that python 3.6 and 3.9 unit tests did not pass. The suggestion was to run the unit tests locally to understand what might be causing the tests to fail.
When the unit tests were run locally, no errors were reported and all the unit tests passed. This meant that there could be a difference in the packages (dependencies) in my dev environment versus what was being installed in the environment that the CI workflow was setting up. Specifically,
The recommendation was to disable python 3.6 unit tests in the CI workflow as python 3.6 is not supported. Further for python 3.9, separately investigate further to identify the differences in dependencies (versions, especially) to understand how the fixes can be made for unit tests to be successful as it is the case when these tests are run locally.
Must complete the following [non-blocking]:
impmodule in matchPolicy.py glideinWMS/glideinwms#300UPDATE
Following are the specific details pertaining to the investigation behind why python3.9 unit tests were failing (these are also included in the commit description):
The error message in the Github action for "Run Python 3.9 unit tests" says:
src/decisionengine_modules/glideinwms/transforms/job_clustering.py:115: AttributeError: module 'pandas.core.computation.ops' has no attribute 'UndefinedVariableError'pandason my dev environment was at version 1.1.5 but pandas installed during the CI workflow was at 1.5.3. For the CI workflow, python 3.9 was installed and due to the dependency requirement defined insetup.py, which ispandas >= 1.1.5; python_version >= '3.7', pandas 2.0.1 was installed for the CI workflow.UndefinedVariableErrorwas defined in thepandas.core.computation.opsmodule. As per the release notes for pandas 1.5.0, starting with 1.5.0,UndefinedVariableError, among others, are changed to be exposed via thepandas.errorsmodule.Unit tests for a few python scripts in the codebase failed with the message
FutureWarning: In a future version of pandas, a length 1 tuple will be returned when iterating over agroupbywith a grouper equal to a list of length 1. Don't supply a list with a single grouper to avoid this warning.This ultimately was resulting in anAssertionErrorin the following unit tests:for collector_host, request_group in requests_df.groupby(["CollectorHost"]):is what was causing the warning to be thrown.assertstatements in the unit testsassertstatements to fail. Because starting pandas 1.5.x, when using a grouper that is equal to a list of length 1, a length-1 tuple is returned rather than a single string element.for ... in ...("col_name"):instead offor ... in ...(["col_name"]):. This returned the keys as strings and not length-1 tuples, which is what the expected output of the unit tests were through theassertstatements.Another message observed was
AttributeError: 'DataFrame' object has no attribute 'append'pandas.DataFrame.append()has been deprecated and with pandas >= 2.0.0, the append method has been completely removed. So, code that referencesappend()reports an AttributeError.concat()method can be used to concatenate multiple dataframes and this was reflected in the filesrc/decisionengine_modules/glideinwms/resource_dist_plugins.py