Xuanwo commented on code in PR #5874: URL: https://github.com/apache/opendal/pull/5874#discussion_r2015787684
########## bindings/ruby/src/layers.rs: ########## @@ -0,0 +1,151 @@ +// 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 +// +// http://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::sync::Arc; +use std::sync::Mutex; +use std::time::Duration; + +use magnus::class; +use magnus::prelude::*; +use magnus::Error; +use magnus::RModule; +use magnus::Ruby; + +use opendal::raw::Accessor as OCoreAccessor; +use opendal::raw::Layer as OCoreLayer; + +use crate::operator::Operator; +use crate::*; + +// Wraps OpenDAL layers +// +// Each layer is represented as a variant of the `Layer` enum. We use this approach because: +// 1. We don't require dynamic plugin registration. +// 2. The set of layers is known at compile time. +// 3. Enums are a native, efficient Rust construct with zero-cost dispatch. +// 4. OpenDAL's layers form a closed set. +// +// Ruby sees this as a class, but Rust handles variant dispatch. magnus also generates additional +// classes to each enum variant. +// +// We’re not using `rb_data_type_t.parent` because we don’t need inheritance-style casting. +// Magnus sets the correct class via `.class_for()` using the enum variant. +// Rust tracks the exact variant in memory; Ruby doesn’t need to know. +#[magnus::wrap(class = "OpenDAL::Layer", free_immediately, size)] +pub enum Layer { + #[magnus(class = "OpenDAL::RetryLayer")] + Retry(Arc<Mutex<ocore::layers::RetryLayer>>), + #[magnus(class = "OpenDAL::ConcurrentLimitLayer")] + ConcurrentLimit(Arc<Mutex<ocore::layers::ConcurrentLimitLayer>>), + #[magnus(class = "OpenDAL::ThrottleLayer")] + Throttle(Arc<Mutex<ocore::layers::ThrottleLayer>>), + #[magnus(class = "OpenDAL::TimeoutLayer")] + Timeout(Arc<Mutex<ocore::layers::TimeoutLayer>>), Review Comment: I somehow wish we can build seperate ruby package like `opendal-layer-timeout` and allow users to use them like native ruby extentions or middleware. This also can avoid compiling everything together. ########## bindings/ruby/src/layers.rs: ########## @@ -0,0 +1,151 @@ +// 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 +// +// http://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::sync::Arc; +use std::sync::Mutex; +use std::time::Duration; + +use magnus::class; +use magnus::prelude::*; +use magnus::Error; +use magnus::RModule; +use magnus::Ruby; + +use opendal::raw::Accessor as OCoreAccessor; +use opendal::raw::Layer as OCoreLayer; + +use crate::operator::Operator; +use crate::*; + +// Wraps OpenDAL layers +// +// Each layer is represented as a variant of the `Layer` enum. We use this approach because: +// 1. We don't require dynamic plugin registration. +// 2. The set of layers is known at compile time. +// 3. Enums are a native, efficient Rust construct with zero-cost dispatch. +// 4. OpenDAL's layers form a closed set. +// +// Ruby sees this as a class, but Rust handles variant dispatch. magnus also generates additional +// classes to each enum variant. +// +// We’re not using `rb_data_type_t.parent` because we don’t need inheritance-style casting. +// Magnus sets the correct class via `.class_for()` using the enum variant. +// Rust tracks the exact variant in memory; Ruby doesn’t need to know. +#[magnus::wrap(class = "OpenDAL::Layer", free_immediately, size)] +pub enum Layer { Review Comment: Can we avoid wrapping them in an enum like this? ########## bindings/ruby/test/blocking_op_test.rb: ########## @@ -103,4 +103,10 @@ class OpenDalTest < ActiveSupport::TestCase io.close end + + test "layer applies a layer" do + @op.layer(OpenDAL::RetryLayer.new) Review Comment: I'm guessing `layer` is not a widely accepted concept in ruby? Do we need to use `middleware` or something? -- 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: commits-unsubscr...@opendal.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
