You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@nifi.apache.org by "martinzink (via GitHub)" <gi...@apache.org> on 2026/08/10 13:02:28 UTC
[PR] MINIFICPP-2870 Lint/Check python code with ruff- #2222 [nifi-minifi-cpp]
martinzink opened a new pull request, #2237:
URL: https://github.com/apache/nifi-minifi-cpp/pull/2237
[MINIFICPP-2870 Lint/Check python code with ruff](https://issues.apache.org/jira/browse/MINIFICPP-2870)
I've deliberetly done the PR in multiple commits, because that would make the review process much easier.
- [ruff format](https://github.com/apache/nifi-minifi-cpp/commit/8eb3f8de99fecfac94964a70894f39f36ecc63de) this is the result of running ruff format from the root of the repo
- [setting ruff rules](https://github.com/apache/nifi-minifi-cpp/commit/186dc748cdae9432f744df42f55df5900d4c6d18) to ignore false positives
- [ruff check --fix](https://github.com/apache/nifi-minifi-cpp/commit/3f097f0dac27795ad16350fdd9f59efad6bc7079) these are the auto fixes from ruff
- [the remaining non-auto fixes](https://github.com/apache/nifi-minifi-cpp/commit/208db2773fb346db1aa988e5ff5899a3d57b08ba) these are the manual fixes (to problems ruff identified)
- [documentation changes](https://github.com/apache/nifi-minifi-cpp/commit/79e14060fda2e47666816d16e076635829d9d8e0)
- [update minimum python to 3.10](https://github.com/apache/nifi-minifi-cpp/commit/ca54c36e5aec7f9c0092db6991cdc7a479b5bc53) I needed to upgrade to python 3.10 because 3.6 didnt support bunch of now standard rules (e.g. list vs List, dict vs Dict), but 3.6 is long EoL
- [extend-exclude](https://github.com/apache/nifi-minifi-cpp/commit/5cc2c8725afeebe727681d605a791d24b812955a) excluded and reverted changed in thirdparty
---
Thank you for submitting a contribution to Apache NiFi - MiNiFi C++.
In order to streamline the review of the contribution we ask you to ensure the following steps have been taken:
### For all changes:
- [ ] Is there a JIRA ticket associated with this PR? Is it referenced in the commit message?
- [ ] Does your PR title start with MINIFICPP-XXXX where XXXX is the JIRA number you are trying to resolve? Pay particular attention to the hyphen "-" character.
- [ ] Has your PR been rebased against the latest commit within the target branch (typically main)?
- [ ] Is your initial contribution a single, squashed commit?
### For code changes:
- [ ] If adding new dependencies to the code, are these dependencies licensed in a way that is compatible for inclusion under [ASF 2.0](http://www.apache.org/legal/resolved.html#category-a)?
- [ ] If applicable, have you updated the LICENSE file?
- [ ] If applicable, have you updated the NOTICE file?
### For documentation related changes:
- [ ] Have you ensured that format looks appropriate for the output in which it is rendered?
### Note:
Please ensure that once the PR is submitted, you check GitHub Actions CI results for build issues and submit an update to your PR as soon as possible.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@nifi.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] MINIFICPP-2870 Lint/Check python code with ruff [nifi-minifi-cpp]
Posted by "szaszm (via GitHub)" <gi...@apache.org>.
szaszm commented on code in PR #2237:
URL: https://github.com/apache/nifi-minifi-cpp/pull/2237#discussion_r3843429301
##########
ruff.toml:
##########
@@ -0,0 +1,15 @@
+line-length = 120
+builtins = ["log", "REL_SUCCESS", "REL_FAILURE", "REL_ORIGINAL", "raw_input"]
+
+extend-exclude = ["thirdparty",
+ "cmake-build-*",
Review Comment:
I'd add an asterisk at the end of build too. I often have build dirs named build-gcc-make or build-clang-ninja
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@nifi.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] MINIFICPP-2870 Lint/Check python code with ruff [nifi-minifi-cpp]
Posted by "fgerlits (via GitHub)" <gi...@apache.org>.
fgerlits commented on code in PR #2237:
URL: https://github.com/apache/nifi-minifi-cpp/pull/2237#discussion_r3843230301
##########
ruff.toml:
##########
@@ -0,0 +1,15 @@
+line-length = 120
+builtins = ["log", "REL_SUCCESS", "REL_FAILURE", "REL_ORIGINAL", "raw_input"]
+
+extend-exclude = ["thirdparty",
+ "cmake-build-*",
+]
Review Comment:
We could include "build", as that is the default build directory name used by the boostrap script.
Also, why did the 80 -> 120 line length change move "thirdparty"? I would move it back to its own line.
```suggestion
extend-exclude = [
"thirdparty",
"build",
"cmake-build-*",
]
```
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@nifi.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] MINIFICPP-2870 Lint/Check python code with ruff [nifi-minifi-cpp]
Posted by "szaszm (via GitHub)" <gi...@apache.org>.
szaszm commented on code in PR #2237:
URL: https://github.com/apache/nifi-minifi-cpp/pull/2237#discussion_r3843440166
##########
ruff.toml:
##########
@@ -0,0 +1,17 @@
+line-length = 120
+builtins = ["log", "REL_SUCCESS", "REL_FAILURE", "REL_ORIGINAL", "raw_input"]
+
+extend-exclude = [
+ "thirdparty",
+ "build",
Review Comment:
```suggestion
"build*",
```
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@nifi.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] MINIFICPP-2870 Lint/Check python code with ruff [nifi-minifi-cpp]
Posted by "martinzink (via GitHub)" <gi...@apache.org>.
martinzink commented on code in PR #2237:
URL: https://github.com/apache/nifi-minifi-cpp/pull/2237#discussion_r3843393402
##########
extensions/sql/tests/features/environment.py:
##########
@@ -16,35 +16,40 @@
#
import os
-import docker
from textwrap import dedent
from containers.postgress_server_container import PostgresContainer
from minifi_behave.containers.docker_image_builder import DockerImageBuilder
-from minifi_behave.core.hooks import common_before_scenario
-from minifi_behave.core.hooks import common_after_scenario
-from minifi_behave.core.hooks import get_minifi_container_image
+from minifi_behave.core.hooks import (
+ common_after_scenario,
+ common_before_scenario,
+ get_minifi_container_image,
+)
+
+import docker
# These hooks are executed by behave before and after each scenario
# The common_before_scenario and common_after_scenario must be called for proper setup and tear down
def before_all(context):
- minifi_tag_prefix = os.environ['MINIFI_TAG_PREFIX'] if 'MINIFI_TAG_PREFIX' in os.environ else ''
+ minifi_tag_prefix = os.environ.get("MINIFI_TAG_PREFIX", "")
if "rocky" in minifi_tag_prefix:
install_sql_cmd = "dnf -y install postgresql-odbc"
so_location = "psqlodbca.so"
- elif "bullseye" in minifi_tag_prefix or "bookworm" in minifi_tag_prefix or "trixie" in minifi_tag_prefix:
- install_sql_cmd = "apt -y install odbc-postgresql"
- so_location = "/usr/lib/$(gcc -dumpmachine)/odbc/psqlodbca.so"
- elif "jammy" in minifi_tag_prefix or "noble" in minifi_tag_prefix:
+ elif (
+ ("bullseye" in minifi_tag_prefix or "bookworm" in minifi_tag_prefix or "trixie" in minifi_tag_prefix)
+ or "jammy" in minifi_tag_prefix
+ or "noble" in minifi_tag_prefix
+ ):
Review Comment:
[review change](https://github.com/apache/nifi-minifi-cpp/pull/2237/commits/a0acca0604129563fa6b42f4af67c1e05cb5491b)
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@nifi.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] MINIFICPP-2870 Lint/Check python code with ruff [nifi-minifi-cpp]
Posted by "martinzink (via GitHub)" <gi...@apache.org>.
martinzink commented on code in PR #2237:
URL: https://github.com/apache/nifi-minifi-cpp/pull/2237#discussion_r3801715766
##########
behave_framework/src/minifi_behave/steps/core_steps.py:
##########
@@ -71,76 +85,138 @@ def __add_directory_with_file_to_container(context: MinifiTestContext, directory
dirs.append(new_dir)
-@step('a directory at "{directory}" has a file with the content "{content}" in the "{flow_name}" flow')
-@step("a directory at '{directory}' has a file with the content '{content}' in the '{flow_name}' flow")
-def create_file_with_content_in_directory_for_flow(context: MinifiTestContext, directory: str, content: str, flow_name: str):
- __add_directory_with_file_to_container(context, directory, str(uuid.uuid4()), content, flow_name)
+@step(
+ 'a directory at "{directory}" has a file with the content "{content}" in the "{flow_name}" flow'
+)
+@step(
+ "a directory at '{directory}' has a file with the content '{content}' in the '{flow_name}' flow"
+)
+def create_file_with_content_in_directory_for_flow(
+ context: MinifiTestContext, directory: str, content: str, flow_name: str
+):
+ __add_directory_with_file_to_container(
+ context, directory, str(uuid.uuid4()), content, flow_name
+ )
Review Comment:
maybe 120?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@nifi.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] MINIFICPP-2870 Lint/Check python code with ruff [nifi-minifi-cpp]
Posted by "martinzink (via GitHub)" <gi...@apache.org>.
martinzink commented on code in PR #2237:
URL: https://github.com/apache/nifi-minifi-cpp/pull/2237#discussion_r3801730331
##########
behave_framework/src/minifi_behave/steps/core_steps.py:
##########
@@ -71,76 +85,138 @@ def __add_directory_with_file_to_container(context: MinifiTestContext, directory
dirs.append(new_dir)
-@step('a directory at "{directory}" has a file with the content "{content}" in the "{flow_name}" flow')
-@step("a directory at '{directory}' has a file with the content '{content}' in the '{flow_name}' flow")
-def create_file_with_content_in_directory_for_flow(context: MinifiTestContext, directory: str, content: str, flow_name: str):
- __add_directory_with_file_to_container(context, directory, str(uuid.uuid4()), content, flow_name)
+@step(
+ 'a directory at "{directory}" has a file with the content "{content}" in the "{flow_name}" flow'
+)
+@step(
+ "a directory at '{directory}' has a file with the content '{content}' in the '{flow_name}' flow"
+)
+def create_file_with_content_in_directory_for_flow(
+ context: MinifiTestContext, directory: str, content: str, flow_name: str
+):
+ __add_directory_with_file_to_container(
+ context, directory, str(uuid.uuid4()), content, flow_name
+ )
Review Comment:
with 120 it would look like this
```
@step('a directory at "{directory}" has a file with the content "{content}" in the "{flow_name}" flow')
@step("a directory at '{directory}' has a file with the content '{content}' in the '{flow_name}' flow")
def create_file_with_content_in_directory_for_flow(
context: MinifiTestContext, directory: str, content: str, flow_name: str
):
__add_directory_with_file_to_container(context, directory, str(uuid.uuid4()), content, flow_name)
```
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@nifi.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] MINIFICPP-2870 Lint/Check python code with ruff [nifi-minifi-cpp]
Posted by "szaszm (via GitHub)" <gi...@apache.org>.
szaszm commented on code in PR #2237:
URL: https://github.com/apache/nifi-minifi-cpp/pull/2237#discussion_r3843208073
##########
extensions/sql/tests/features/environment.py:
##########
@@ -16,35 +16,40 @@
#
import os
-import docker
from textwrap import dedent
from containers.postgress_server_container import PostgresContainer
from minifi_behave.containers.docker_image_builder import DockerImageBuilder
-from minifi_behave.core.hooks import common_before_scenario
-from minifi_behave.core.hooks import common_after_scenario
-from minifi_behave.core.hooks import get_minifi_container_image
+from minifi_behave.core.hooks import (
+ common_after_scenario,
+ common_before_scenario,
+ get_minifi_container_image,
+)
+
+import docker
# These hooks are executed by behave before and after each scenario
# The common_before_scenario and common_after_scenario must be called for proper setup and tear down
def before_all(context):
- minifi_tag_prefix = os.environ['MINIFI_TAG_PREFIX'] if 'MINIFI_TAG_PREFIX' in os.environ else ''
+ minifi_tag_prefix = os.environ.get("MINIFI_TAG_PREFIX", "")
if "rocky" in minifi_tag_prefix:
install_sql_cmd = "dnf -y install postgresql-odbc"
so_location = "psqlodbca.so"
- elif "bullseye" in minifi_tag_prefix or "bookworm" in minifi_tag_prefix or "trixie" in minifi_tag_prefix:
- install_sql_cmd = "apt -y install odbc-postgresql"
- so_location = "/usr/lib/$(gcc -dumpmachine)/odbc/psqlodbca.so"
- elif "jammy" in minifi_tag_prefix or "noble" in minifi_tag_prefix:
+ elif (
+ ("bullseye" in minifi_tag_prefix or "bookworm" in minifi_tag_prefix or "trixie" in minifi_tag_prefix)
+ or "jammy" in minifi_tag_prefix
+ or "noble" in minifi_tag_prefix
+ ):
Review Comment:
The parens from the line with "bullseye", "bookworm", and "trixie" could be removed.
##########
ruff.toml:
##########
@@ -0,0 +1,15 @@
+line-length = 120
+builtins = ["log", "REL_SUCCESS", "REL_FAILURE", "REL_ORIGINAL", "raw_input"]
+
+extend-exclude = ["thirdparty",
+ "cmake-build-*",
Review Comment:
```suggestion
"cmake-build-*",
"build*",
```
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@nifi.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] MINIFICPP-2870 Lint/Check python code with ruff [nifi-minifi-cpp]
Posted by "martinzink (via GitHub)" <gi...@apache.org>.
martinzink commented on code in PR #2237:
URL: https://github.com/apache/nifi-minifi-cpp/pull/2237#discussion_r3843663874
##########
ruff.toml:
##########
@@ -0,0 +1,15 @@
+line-length = 120
+builtins = ["log", "REL_SUCCESS", "REL_FAILURE", "REL_ORIGINAL", "raw_input"]
+
+extend-exclude = ["thirdparty",
+ "cmake-build-*",
Review Comment:
https://github.com/apache/nifi-minifi-cpp/pull/2237/commits/d4c9e12f1a295e3d65d1083eb45037098866cad1
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@nifi.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] MINIFICPP-2870 Lint/Check python code with ruff [nifi-minifi-cpp]
Posted by "szaszm (via GitHub)" <gi...@apache.org>.
szaszm commented on code in PR #2237:
URL: https://github.com/apache/nifi-minifi-cpp/pull/2237#discussion_r3796396546
##########
behave_framework/src/minifi_behave/containers/container_linux.py:
##########
@@ -333,34 +388,46 @@ def _get_contents_of_all_files_in_directory(self, directory_path: str) -> list[s
exit_code, content = self.exec_run(read_command)
if exit_code != 0:
- error_message = f"Command to read file '{path}' failed with exit code {exit_code}"
- logging.error(error_message)
+ error_message = (
+ f"Command to read file '{path}' failed with exit code {exit_code}"
+ )
Review Comment:
I find this reformatting weird, why change it from a single line?
Since it's automated reformatting, I don't mind it, just wanted to bring some attention to this.
##########
behave_framework/src/minifi_behave/steps/core_steps.py:
##########
@@ -71,76 +85,138 @@ def __add_directory_with_file_to_container(context: MinifiTestContext, directory
dirs.append(new_dir)
-@step('a directory at "{directory}" has a file with the content "{content}" in the "{flow_name}" flow')
-@step("a directory at '{directory}' has a file with the content '{content}' in the '{flow_name}' flow")
-def create_file_with_content_in_directory_for_flow(context: MinifiTestContext, directory: str, content: str, flow_name: str):
- __add_directory_with_file_to_container(context, directory, str(uuid.uuid4()), content, flow_name)
+@step(
+ 'a directory at "{directory}" has a file with the content "{content}" in the "{flow_name}" flow'
+)
+@step(
+ "a directory at '{directory}' has a file with the content '{content}' in the '{flow_name}' flow"
+)
+def create_file_with_content_in_directory_for_flow(
+ context: MinifiTestContext, directory: str, content: str, flow_name: str
+):
+ __add_directory_with_file_to_container(
+ context, directory, str(uuid.uuid4()), content, flow_name
+ )
Review Comment:
same, we're expanding 4 lines into 11 slightly shorter lines, I don't think this improves readability, but if the python community consensus suggests this, then I won't complain
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@nifi.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] MINIFICPP-2870 Lint/Check python code with ruff [nifi-minifi-cpp]
Posted by "martinzink (via GitHub)" <gi...@apache.org>.
martinzink commented on code in PR #2237:
URL: https://github.com/apache/nifi-minifi-cpp/pull/2237#discussion_r3801714106
##########
behave_framework/src/minifi_behave/steps/core_steps.py:
##########
@@ -71,76 +85,138 @@ def __add_directory_with_file_to_container(context: MinifiTestContext, directory
dirs.append(new_dir)
-@step('a directory at "{directory}" has a file with the content "{content}" in the "{flow_name}" flow')
-@step("a directory at '{directory}' has a file with the content '{content}' in the '{flow_name}' flow")
-def create_file_with_content_in_directory_for_flow(context: MinifiTestContext, directory: str, content: str, flow_name: str):
- __add_directory_with_file_to_container(context, directory, str(uuid.uuid4()), content, flow_name)
+@step(
+ 'a directory at "{directory}" has a file with the content "{content}" in the "{flow_name}" flow'
+)
+@step(
+ "a directory at '{directory}' has a file with the content '{content}' in the '{flow_name}' flow"
+)
+def create_file_with_content_in_directory_for_flow(
+ context: MinifiTestContext, directory: str, content: str, flow_name: str
+):
+ __add_directory_with_file_to_container(
+ context, directory, str(uuid.uuid4()), content, flow_name
+ )
Review Comment:
i think it comes from the deault ruff line-length (88 by default), we can adjust this i have nothing againts that
https://docs.astral.sh/ruff/settings/#line-length
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@nifi.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] MINIFICPP-2870 Lint/Check python code with ruff [nifi-minifi-cpp]
Posted by "martinzink (via GitHub)" <gi...@apache.org>.
martinzink commented on code in PR #2237:
URL: https://github.com/apache/nifi-minifi-cpp/pull/2237#discussion_r3843378290
##########
ruff.toml:
##########
@@ -0,0 +1,15 @@
+line-length = 120
+builtins = ["log", "REL_SUCCESS", "REL_FAILURE", "REL_ORIGINAL", "raw_input"]
+
+extend-exclude = ["thirdparty",
+ "cmake-build-*",
Review Comment:
https://github.com/apache/nifi-minifi-cpp/pull/2237/commits/1018dc5f2de30a04f5de34dd80a5547b3f73975b
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@nifi.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] MINIFICPP-2870 Lint/Check python code with ruff [nifi-minifi-cpp]
Posted by "martinzink (via GitHub)" <gi...@apache.org>.
martinzink merged PR #2237:
URL: https://github.com/apache/nifi-minifi-cpp/pull/2237
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@nifi.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org