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.

Sign up for GitHub

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

Jump to bottom

Add complexity metrics and enable checks in pre-commit #525

Closed
3 tasks
tatiana opened this issue Sep 11, 2023 · 0 comments · Fixed by #629
Closed
3 tasks

Add complexity metrics and enable checks in pre-commit #525

tatiana opened this issue Sep 11, 2023 · 0 comments · Fixed by #629
Labels
good first issue Good for newcomers

Comments

@tatiana
Copy link
Collaborator

tatiana commented Sep 11, 2023 •
edited
Loading

We should add metrics for cyclomatic complexity in Cosmos, to ensure the code is not becoming overly complicated - and set a threshold, having pre-commit checking this. A way of accomplishing this would be to adopt Radon/Zenon or McCabe:

https://github.com/rubik/radon
https://github.com/yunojuno/pre-commit-xenon
https://pypi.org/project/mccabe/

Acceptance criteria:

  • Enable code complexity check in pre-commit hook
  • Configure complexity threshold to 10
  • Fix existing code to be compliant with threshold set
@tatiana tatiana added the good first issue Good for newcomers label Sep 11, 2023
@jbandoro jbandoro mentioned this issue Oct 26, 2023
Merged
2 tasks
@tatiana tatiana mentioned this issue Nov 2, 2023
Closed
tatiana pushed a commit that referenced this issue Nov 2, 2023
@jbandoro
Add pre-commit hook for McCabe max complexity check and fix errors (#629
f9809a8 
)

Reduce Cosmos code complexity from 18 to 10, automating checks as part of the CI.

```shell
❯ pre-commit run flake8 --all-files
flake8...................................................................Failed
- hook id: flake8
- exit code: 1

cosmos/dbt/graph.py:134:5: C901 'DbtGraph.load_via_dbt_ls' is too complex (16)
cosmos/dbt/parser/project.py:136:5: C901 'DbtModel.__post_init__' is too complex (18)
cosmos/dbt/parser/project.py:346:5: C901 'LegacyDbtProject._handle_config_file' is too complex (15)
cosmos/dbt/selector.py:87:1: C901 'select_nodes_ids_by_intersection' is too complex (16)
```

Closes: #525
tatiana pushed a commit that referenced this issue Nov 6, 2023
@jbandoro @tatiana
Add pre-commit hook for McCabe max complexity check and fix errors (#629
067bf3a 
)

Reduce Cosmos code complexity from 18 to 10, automating checks as part of the CI.

```shell
❯ pre-commit run flake8 --all-files
flake8...................................................................Failed
- hook id: flake8
- exit code: 1

cosmos/dbt/graph.py:134:5: C901 'DbtGraph.load_via_dbt_ls' is too complex (16)
cosmos/dbt/parser/project.py:136:5: C901 'DbtModel.__post_init__' is too complex (18)
cosmos/dbt/parser/project.py:346:5: C901 'LegacyDbtProject._handle_config_file' is too complex (15)
cosmos/dbt/selector.py:87:1: C901 'select_nodes_ids_by_intersection' is too complex (16)
```

Closes: #525
(cherry picked from commit f9809a8)
tatiana pushed a commit that referenced this issue Nov 6, 2023
@jbandoro @tatiana
Add pre-commit hook for McCabe max complexity check and fix errors (#629
166e2bc 
)

Reduce Cosmos code complexity from 18 to 10, automating checks as part of the CI.

```shell
❯ pre-commit run flake8 --all-files
flake8...................................................................Failed
- hook id: flake8
- exit code: 1

cosmos/dbt/graph.py:134:5: C901 'DbtGraph.load_via_dbt_ls' is too complex (16)
cosmos/dbt/parser/project.py:136:5: C901 'DbtModel.__post_init__' is too complex (18)
cosmos/dbt/parser/project.py:346:5: C901 'LegacyDbtProject._handle_config_file' is too complex (15)
cosmos/dbt/selector.py:87:1: C901 'select_nodes_ids_by_intersection' is too complex (16)
```

Closes: #525
(cherry picked from commit f9809a8)
@joppevos joppevos mentioned this issue Dec 3, 2023
Merged
2 tasks
tatiana pushed a commit that referenced this issue Dec 5, 2023
@joppevos
Reduce Mccabe code complexity to 8 (#738)
6ccde5c 
Reduce the maccabe score from 10 to 8 and refactored the code flagged as too complex.

I removed flake8, since we can use ruffs maccabe linter.

The following functions have been addressed
```shell
> pre-commit run flake8 --all-files
flake8...................................................................Failed
- hook id: flake8
- exit code: 1

cosmos/airflow/graph.py:196:1: C901 'build_airflow_graph' is too complex (9)
cosmos/converter.py:101:5: C901 'DbtToAirflowConverter.__init__' is too complex (9)
cosmos/dbt/parser/project.py:277:5: C901 'LegacyDbtProject.__post_init__' is too complex (10)
cosmos/dbt/selector.py:197:1: C901 'select_nodes' is too complex (9)

```
I picked out the functions which stood out to me as complex. If we want
to reduce it to 6, there are plenty more functions to tackle. For some
functions I would consider it unnecessary to drop it to 6.
```shell
cosmos/config.py:108:9: C901 `__init__` is too complex (7 > 6)
cosmos/dbt/parser/project.py:63:9: C901 `_config_selector_ooo` is too complex (7 > 6)
cosmos/dbt/parser/project.py:98:5: C901 `extract_python_file_upstream_requirements` is too complex (7 > 6)
cosmos/dbt/parser/project.py:165:9: C901 `extract_sql_file_requirements` is too complex (7 > 6)
cosmos/dbt/selector.py:52:9: C901 `load_from_statement` is too complex (7 > 6)
cosmos/dbt/selector.py:119:9: C901 `_should_include_node` is too complex (7 > 6)
cosmos/hooks/subprocess.py:34:9: C901 `run_command` is too complex (8 > 6)
cosmos/operators/base.py:137:9: C901 `get_env` is too complex (7 > 6)
cosmos/operators/base.py:186:9: C901 `add_global_flags` is too complex (7 > 6)
cosmos/operators/local.py:136:9: C901 `store_compiled_sql` is too complex (7 > 6)
cosmos/profiles/base.py:183:9: C901 `get_dbt_value` is too complex (8 > 6)
Found 11 errors.
```

This PR continues on the amazing work of @jbandoro on #525 
Related issue #641
arojasb3 pushed a commit to arojasb3/astronomer-cosmos that referenced this issue Jul 14, 2024
@joppevos @arojasb3
Reduce Mccabe code complexity to 8 (astronomer#738)
f5274b1 
Reduce the maccabe score from 10 to 8 and refactored the code flagged as too complex.

I removed flake8, since we can use ruffs maccabe linter.

The following functions have been addressed
```shell
> pre-commit run flake8 --all-files
flake8...................................................................Failed
- hook id: flake8
- exit code: 1

cosmos/airflow/graph.py:196:1: C901 'build_airflow_graph' is too complex (9)
cosmos/converter.py:101:5: C901 'DbtToAirflowConverter.__init__' is too complex (9)
cosmos/dbt/parser/project.py:277:5: C901 'LegacyDbtProject.__post_init__' is too complex (10)
cosmos/dbt/selector.py:197:1: C901 'select_nodes' is too complex (9)

```
I picked out the functions which stood out to me as complex. If we want
to reduce it to 6, there are plenty more functions to tackle. For some
functions I would consider it unnecessary to drop it to 6.
```shell
cosmos/config.py:108:9: C901 `__init__` is too complex (7 > 6)
cosmos/dbt/parser/project.py:63:9: C901 `_config_selector_ooo` is too complex (7 > 6)
cosmos/dbt/parser/project.py:98:5: C901 `extract_python_file_upstream_requirements` is too complex (7 > 6)
cosmos/dbt/parser/project.py:165:9: C901 `extract_sql_file_requirements` is too complex (7 > 6)
cosmos/dbt/selector.py:52:9: C901 `load_from_statement` is too complex (7 > 6)
cosmos/dbt/selector.py:119:9: C901 `_should_include_node` is too complex (7 > 6)
cosmos/hooks/subprocess.py:34:9: C901 `run_command` is too complex (8 > 6)
cosmos/operators/base.py:137:9: C901 `get_env` is too complex (7 > 6)
cosmos/operators/base.py:186:9: C901 `add_global_flags` is too complex (7 > 6)
cosmos/operators/local.py:136:9: C901 `store_compiled_sql` is too complex (7 > 6)
cosmos/profiles/base.py:183:9: C901 `get_dbt_value` is too complex (8 > 6)
Found 11 errors.
```

This PR continues on the amazing work of @jbandoro on astronomer#525 
Related issue astronomer#641
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
good first issue Good for newcomers
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

Add pre-commit hook for McCabe max complexity check and fix errors jbandoro/astronomer-cosmos
1 participant
@tatiana