fgerlits commented on code in PR #2186:
URL: https://github.com/apache/nifi-minifi-cpp/pull/2186#discussion_r3613561209
##########
minifi_rust/extensions/minifi_rs_playground/features/steps/steps.py:
##########
@@ -0,0 +1,45 @@
+from behave import then, when, given
+import humanfriendly
+
+from minifi_behave.steps import checking_steps # noqa: F401
+from minifi_behave.steps import configuration_steps # noqa: F401
+from minifi_behave.steps import core_steps # noqa: F401
+from minifi_behave.steps import flow_building_steps # noqa: F401
+from minifi_behave.core.helpers import wait_for_condition
+from minifi_behave.core.minifi_test_context import MinifiTestContext
+
+
+@when("the MiNiFi instance is started without assertions")
+def minifi_starts_wo_assertions(context: MinifiTestContext):
+ context.get_or_create_default_minifi_container().deploy(context)
Review Comment:
how is this different from the standard "the MiNiFi instance starts up" step?
##########
minifi_rust/minifi_native/src/api/property.rs:
##########
@@ -0,0 +1,117 @@
+// Licensed to the Apache Software Foundation (ASF) under one
+// or more contributor license agreements. See the NOTICE file
+// distributed with this work for additional information
+// regarding copyright ownership. The ASF licenses this file
+// to you under the Apache License, Version 2.0 (the
+// "License"); you may not use this file except in compliance
+// with the License. You may obtain a copy of the License at
+//
+// https://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing,
+// software distributed under the License is distributed on an
+// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+// KIND, either express or implied. See the License for the
+// specific language governing permissions and limitations
+// under the License.
+
+use crate::StandardPropertyValidator::{
+ BoolValidator, DataSizeValidator, TimePeriodValidator, U64Validator,
+};
+use crate::{
+ ComponentIdentifier, ControllerServiceDefinition, EnableControllerService,
MinifiError,
+};
+use std::str::FromStr;
+use std::time::Duration;
+
+#[derive(Debug, Eq, PartialEq)]
+pub enum StandardPropertyValidator {
+ AlwaysValidValidator,
+ NonBlankValidator,
+ TimePeriodValidator,
+ BoolValidator,
+ I64Validator,
+ U64Validator,
+ DataSizeValidator,
+ PortValidator,
+}
+
+#[derive(Debug)]
+pub struct Property {
+ pub name: &'static str,
+ pub description: &'static str,
+ pub is_required: bool,
+ pub is_sensitive: bool,
+ pub supports_expr_lang: bool,
+ pub default_value: Option<&'static str>,
+ pub validator: StandardPropertyValidator,
+ pub allowed_values: &'static [&'static str],
Review Comment:
Should `allowed_values` be an `Option`, too? I think it would make more
sense to use `None` to mean "no restrictions", because an empty slice looks
like "no values are allowed".
I don't know if the type system can represent "optional slice of strings,
but if it is Some, then it is non-empty", but if it can, that would be really
nice.
##########
minifi_rust/extensions/minifi_rs_playground/src/processors/get_file/relationships.rs:
##########
@@ -0,0 +1,23 @@
+// Licensed to the Apache Software Foundation (ASF) under one
+// or more contributor license agreements. See the NOTICE file
+// distributed with this work for additional information
+// regarding copyright ownership. The ASF licenses this file
+// to you under the Apache License, Version 2.0 (the
+// "License"); you may not use this file except in compliance
+// with the License. You may obtain a copy of the License at
+//
+// https://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing,
+// software distributed under the License is distributed on an
+// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+// KIND, either express or implied. See the License for the
+// specific language governing permissions and limitations
+// under the License.
+
+use minifi_native::Relationship;
+
+pub(crate) const SUCCESS: Relationship = Relationship {
+ name: "success",
+ description: "FlowFiles are transferred here after logging",
Review Comment:
nitpicking:
```suggestion
description: "The created FlowFiles are transferred here",
```
##########
minifi_rust/extensions/minifi_rs_playground/src/controller_services/lorem_ipsum_controller_service/properties.rs:
##########
@@ -0,0 +1,13 @@
+use minifi_native::{Property, StandardPropertyValidator};
+
+pub(crate) const LENGTH: Property = Property {
Review Comment:
When we are using the multiple-file style, can we put the processor
definition at the parent level and move the implementation to the subdirectory?
That would make more sense to me, since the processor definition file is
similar to a table of contents, and it also contains the description of the
processor.
##########
minifi_rust/minifi_native_sys/src/lib.rs:
##########
@@ -0,0 +1,28 @@
+// Licensed to the Apache Software Foundation (ASF) under one
+// or more contributor license agreements. See the NOTICE file
+// distributed with this work for additional information
+// regarding copyright ownership. The ASF licenses this file
+// to you under the Apache License, Version 2.0 (the
+// "License"); you may not use this file except in compliance
+// with the License. You may obtain a copy of the License at
+//
+// https://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing,
+// software distributed under the License is distributed on an
+// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+// KIND, either express or implied. See the License for the
+// specific language governing permissions and limitations
+// under the License.
+
+// minifi-sys/src/lib.rs
Review Comment:
stale comment?
```suggestion
```
##########
minifi_rust/minifi_native_sys/build.rs:
##########
@@ -0,0 +1,240 @@
+// Licensed to the Apache Software Foundation (ASF) under one
+// or more contributor license agreements. See the NOTICE file
+// distributed with this work for additional information
+// regarding copyright ownership. The ASF licenses this file
+// to you under the Apache License, Version 2.0 (the
+// "License"); you may not use this file except in compliance
+// with the License. You may obtain a copy of the License at
+//
+// https://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing,
+// software distributed under the License is distributed on an
+// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+// KIND, either express or implied. See the License for the
+// specific language governing permissions and limitations
+// under the License.
+
+use std::env;
+use std::path::{Path, PathBuf};
+use std::process::Command;
+
+struct Sdk {
+ header_path: PathBuf,
+ #[cfg_attr(not(windows), allow(dead_code))]
+ def_path: PathBuf,
+ behave_path: PathBuf,
+}
+
+impl Sdk {
+ fn from_local_repo(repo_root: &Path) -> Option<Self> {
+ let canon = std::fs::canonicalize(repo_root).ok()?;
+ println!(
+ "cargo:info=MINIFI_SDK_PATH from from_local_repo: {:?} -> {:?}",
+ repo_root, canon
+ );
+
+ let header_path =
canon.to_path_buf().join("minifi-api/include/minifi-api.h");
+ let def_path = canon.to_path_buf().join("minifi-api/minifi-api.def");
+ let behave_path = canon.to_path_buf().join("behave_framework");
+
+ println!(
+ "cargo:info=header_path {:?}\n def_path {:?}\n behave_path {:?}",
+ header_path, def_path, behave_path
+ );
+
+ if !header_path.exists() || !def_path.exists() {
+ return None;
+ }
+
+ Some(Self {
+ header_path,
+ def_path,
+ behave_path,
+ })
+ }
+
+ fn from_local_sdk_path(sdk_path: &Path) -> Option<Self> {
+ let base_path = if sdk_path.join("minifi-api.h").exists() {
+ sdk_path.to_path_buf()
+ } else if sdk_path.join("minifi-native-sdk/minifi-api.h").exists() {
+ sdk_path.join("minifi-native-sdk")
+ } else {
+ return None;
+ };
+
+ let header_path = base_path.join("minifi-api.h");
+ let def_path = base_path.join("minifi-api.def");
+ let behave_path = std::fs::read_dir(&base_path)
+ .ok()?
+ .filter_map(|e| e.ok())
+ .find(|e| e.path().extension().unwrap_or_default() == "whl")
+ .map(|e| e.path())?;
+
+ if !header_path.exists() || !def_path.exists() ||
!behave_path.exists() {
+ return None;
+ }
+
+ Some(Self {
+ header_path,
+ def_path,
+ behave_path,
+ })
+ }
+
+ fn new_from_env_variable() -> Option<Self> {
+ let out_dir = PathBuf::from(env::var("OUT_DIR").unwrap());
+ let sdk_path = env::var("MINIFI_SDK_PATH").expect(
+ "MINIFI_SDK_PATH must be set. Please define it in your environment
or .cargo/config.toml"
+ );
+
+ println!("cargo:info=Using MINIFI_SDK_PATH: {}", sdk_path);
+
+ if sdk_path.starts_with("https://";) {
+ let zip_path = out_dir.join("downloaded-sdk.zip");
+ let extract_dir = out_dir.join("extracted-sdk");
+ Self::ensure_extracted(&extract_dir, || {
+ Self::download_file(&sdk_path, &zip_path);
+ Self::extract_zip(&zip_path, &extract_dir);
+ });
+ Self::from_local_sdk_path(&extract_dir)
+ } else {
+ let local_path = if sdk_path.starts_with(".") {
+ PathBuf::from(format!("../{}", sdk_path))
+ } else {
+ PathBuf::from(&sdk_path)
+ };
+ if local_path.is_file() &&
local_path.extension().unwrap_or_default() == "zip" {
+ let extract_dir = out_dir.join("extracted-local-sdk");
+ Self::ensure_extracted(&extract_dir, || {
+ Self::extract_zip(&local_path, &extract_dir);
+ });
+ Self::from_local_sdk_path(&extract_dir)
+ } else {
+ Self::from_local_sdk_path(&local_path)
+ .or_else(|| Self::from_local_repo(&local_path))
+ }
+ }
+ }
+
+ /// Run `extract` unless a previous run completed successfully (marked by a
+ /// sentinel file inside `dest_dir`). A partial extraction — for example if
+ /// `curl`/`tar` panicked mid-way — leaves `dest_dir` on disk without the
+ /// sentinel, so the next build reruns the extraction rather than working
+ /// with a broken tree.
+ fn ensure_extracted<F: FnOnce()>(dest_dir: &Path, extract: F) {
+ let sentinel = dest_dir.join(".extraction-complete");
+ if sentinel.exists() {
+ return;
+ }
+ if dest_dir.exists() {
+ std::fs::remove_dir_all(dest_dir).unwrap_or_else(|e| {
+ panic!(
+ "Failed to clean up stale extraction directory {:?}: {}",
+ dest_dir, e
+ )
+ });
+ }
+ extract();
+ std::fs::write(&sentinel, b"ok").unwrap_or_else(|e| {
+ panic!("Failed to write extraction sentinel {:?}: {}", sentinel, e)
+ });
+ }
+
+ fn download_file(url: &str, dest: &Path) {
+ println!("cargo:warning=Downloading SDK from {}...", url);
+ let status = Command::new("curl")
+ .args(["-fSL", "-o", dest.to_str().unwrap(), url])
+ .status()
+ .expect("Failed to execute curl to download SDK");
+ assert!(status.success(), "Failed to download SDK from {}", url);
+ }
+
+ fn extract_zip(archive: &Path, dest_dir: &Path) {
+ println!("cargo:warning=Extracting SDK archive...");
+ std::fs::create_dir_all(dest_dir).unwrap();
+ let status = Command::new("tar")
+ .args([
+ "-xf",
+ archive.to_str().unwrap(),
+ "-C",
+ dest_dir.to_str().unwrap(),
+ ])
+ .status()
+ .expect("Failed to execute tar to extract SDK zip");
+ assert!(status.success(), "Failed to extract SDK zip file");
+ }
+}
+
+#[cfg(windows)]
+fn generate_minifi_c_api_lib(def_path: &Path) {
+ let tool = cc::Build::new()
+ .try_get_compiler()
+ .expect("Failed to find the MSVC toolchain. Is Visual Studio or the
C++ Build Tools workload installed?");
+
+ let lib_exe_path = tool
+ .path()
+ .parent()
+ .expect("Compiler path is expected to be in a 'bin' directory.")
+ .join("lib.exe");
+
+ if !lib_exe_path.exists() {
+ panic!(
+ "Could not find lib.exe at the expected path: {}",
+ lib_exe_path.display()
+ );
+ }
+
+ let out_dir = PathBuf::from(env::var("OUT_DIR").unwrap());
+ let lib_out_path = out_dir.join("minifi-api.lib");
+
+ let status = Command::new(&lib_exe_path)
+ .arg(format!("/def:{}", def_path.display()))
+ .arg(format!("/out:{}", lib_out_path.display()))
+ .arg(format!(
+ "/machine:{}",
+ env::var("CARGO_CFG_TARGET_ARCH").unwrap().to_uppercase()
+ ))
+ .status()
+ .expect("Failed to execute lib.exe.");
+
+ if !status.success() {
+ panic!("lib.exe failed to create the import library from the .def
file.");
+ }
+
+ println!("cargo:rustc-link-lib=dylib=minifi-api");
+ println!("cargo:rustc-link-search=native={}", out_dir.display());
+}
+
+fn main() {
+ println!("cargo:rerun-if-env-changed=MINIFI_SDK_PATH");
+
+ let sdk = Sdk::new_from_env_variable()
+ .expect("Couldn't find valid SDK. Ensure MINIFI_SDK_PATH is set
correctly.");
+
+ #[cfg(windows)]
+ generate_minifi_c_api_lib(&sdk.def_path);
+
+ println!("cargo:rerun-if-changed={}", sdk.header_path.display());
+ println!("cargo:rerun-if-changed={}", sdk.def_path.display());
+ let behave_path_canonical =
sdk.behave_path.canonicalize().unwrap_or_else(|e| {
+ panic!(
+ "Failed to canonicalize behave path {:?}: {}. Ensure the behave
framework \
+ is present alongside the SDK (set MINIFI_SDK_PATH to a full SDK
checkout).",
+ sdk.behave_path, e
+ )
+ });
+ println!("cargo:behave_path={}", behave_path_canonical.display());
+
+ let bindings = bindgen::Builder::default()
+ .header(sdk.header_path.to_str().unwrap())
+ .clang_arg("-std=c2x")
Review Comment:
Can this `"-std=c2x"` come from the `Cargo.toml` file? It looks like
something which will need regular updates, and it will be easy to forget if it
is hidden here.
--
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: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
