Feat: Implement client to interact with Operaton's API-REST #142
Conversation
WalkthroughA new Ruby class, Changes
Sequence Diagram(s)sequenceDiagram
participant Client as ExternalTaskClient
participant API as External Task API
Client->>API: fetch_and_lock(topics, lock_duration, ...)
API-->>Client: List of locked tasks
Client->>API: complete(task_id, variables)
API-->>Client: Completion confirmation
Client->>API: get_variables(task_id)
API-->>Client: Task variables
Client->>API: unlock(task_id)
API-->>Client: Unlock confirmation
Client->>API: report_failure(task_id, error_message, ...)
API-->>Client: Failure reported confirmation
Poem
📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 4
🧹 Nitpick comments (4)
lib/bas/utils/operaton/external_task_client.rb (4)
21-38: Consider extracting topic formatting logic.The method exceeds the recommended length limit and mixes input validation with business logic.
Extract the topic formatting logic into a private method:
def fetch_and_lock(topics_str, lock_duration: 10_000, max_tasks: 1, use_priority: true, variables: []) - topic_names = topics_str.is_a?(Array) ? topics_str : topics_str.to_s.split(',') - - formatted_topics = topic_names.map do |name| - { - topicName: name.strip, - lockDuration: lock_duration, - variables: variables - } - end - post("/external-task/fetchAndLock", { workerId: @worker_id, maxTasks: max_tasks, usePriority: use_priority, - topics: formatted_topics + topics: format_topics(topics_str, lock_duration, variables) }) end +private + +def format_topics(topics_str, lock_duration, variables) + topic_names = topics_str.is_a?(Array) ? topics_str : topics_str.to_s.split(',') + topic_names.map do |name| + { + topicName: name.strip, + lockDuration: lock_duration, + variables: variables + } + end +end
42-44: Fix hash indentation.The hash indentation is inconsistent with Ruby style guidelines.
post("/external-task/#{task_id}/complete", { - workerId: @worker_id, - variables: format_variables(variables) -}) + workerId: @worker_id, + variables: format_variables(variables) + })
57-62: Fix hash indentation.The hash indentation is inconsistent with Ruby style guidelines.
post("/external-task/#{task_id}/failure", { - workerId: @worker_id, - errorMessage: error_message, - errorDetails: error_details, - retries: retries, - retryTimeout: retry_timeout -}) + workerId: @worker_id, + errorMessage: error_message, + errorDetails: error_details, + retries: retries, + retryTimeout: retry_timeout + })
96-104: Consider edge cases in type mapping.The type mapping doesn't handle some Ruby types that might be commonly used.
Consider adding support for additional Ruby types:
def ruby_type_to_operaton_type(value) case value when String then "String" when Integer then "Integer" when Float then "Double" when TrueClass, FalseClass then "Boolean" + when NilClass then "Null" + when Array, Hash then "Object" + when Date, Time, DateTime then "String" else "Object" end end
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
lib/bas/utils/operaton/external_task_client.rb(1 hunks)
🧰 Additional context used
🪛 RuboCop (1.75.5)
lib/bas/utils/operaton/external_task_client.rb
[convention] 9-9: Missing top-level documentation comment for class Bas::Utils::Operaton::ExternalTaskClient.
(Style/Documentation)
[convention] 21-38: Method has too many lines. [14/10]
(Metrics/MethodLength)
[convention] 22-22: Prefer double-quoted strings unless you need single quotes to avoid extra backslashes for escaping.
(Style/StringLiterals)
[convention] 42-42: Use 2 spaces for indentation in a hash, relative to the first position after the preceding left parenthesis.
(Layout/FirstHashElementIndentation)
[convention] 44-44: Indent the right brace the same as the first position after the preceding left parenthesis.
(Layout/FirstHashElementIndentation)
[convention] 57-57: Use 2 spaces for indentation in a hash, relative to the first position after the preceding left parenthesis.
(Layout/FirstHashElementIndentation)
[convention] 62-62: Indent the right brace the same as the first position after the preceding left parenthesis.
(Layout/FirstHashElementIndentation)
[convention] 80-80: Use a guard clause (raise "Operaton API Error #{response.status}: #{response.body}" unless response.success?) instead of wrapping the code inside a conditional expression.
(Style/GuardClause)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: Ruby 3.2.2
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (1)
Gemfile (1)
20-20: Match string-literal style used elsewhereEvery other gem declaration uses double quotes; switch here to silence Style/StringLiterals warnings.
-gem 'json', '~> 2.8' +gem "json", "~> 2.8"
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
Gemfile(1 hunks)
🧰 Additional context used
🪛 RuboCop (1.75.5)
Gemfile
[convention] 20-20: Prefer double-quoted strings unless you need single quotes to avoid extra backslashes for escaping.
(Style/StringLiterals)
[convention] 20-20: Prefer double-quoted strings unless you need single quotes to avoid extra backslashes for escaping.
(Style/StringLiterals)
Pull Request Test Coverage Report for Build 16128544331Details
💛 - Coveralls |
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (3)
Gemfile (2)
18-18: Pin Faraday to a stable series.
20-20:jsongem is usually unnecessary and can introduce ABI conflicts.lib/bas/utils/operaton/external_task_client.rb (1)
90-94: Improve error handling to avoid sensitive information exposure.The current error handling exposes potentially sensitive information from the response body, which could be a security concern.
def handle_response(response) - raise "Operaton API Error #{response.status}: #{response.body}" unless response.success? + raise "Operaton API Error #{response.status}: #{response.reason_phrase}" unless response.success? response.body endThis change:
- Avoids exposing potentially sensitive response body content
- Uses
reason_phrasefor more appropriate error context- Maintains debugging capability without security risks
🧹 Nitpick comments (1)
lib/bas/utils/operaton/external_task_client.rb (1)
35-41: Consider adding parameter validation for fetch_and_lock method.The method has good parameter defaults, but consider validating critical parameters to prevent runtime errors.
def fetch_and_lock(topics_str, lock_duration: 10_000, max_tasks: 1, use_priority: true, variables: []) + raise ArgumentError, "topics_str cannot be nil or empty" if topics_str.nil? || (topics_str.respond_to?(:empty?) && topics_str.empty?) + raise ArgumentError, "lock_duration must be positive" if lock_duration <= 0 + raise ArgumentError, "max_tasks must be positive" if max_tasks <= 0 + post("/external-task/fetchAndLock", workerId: @worker_id, maxTasks: max_tasks, usePriority: use_priority, topics: build_topics_payload(topics_str, lock_duration, variables)) end
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
Gemfile(1 hunks)lib/bas/utils/operaton/external_task_client.rb(1 hunks)
🧰 Additional context used
🪛 GitHub Actions: Ruby
Gemfile
[error] 18-18: Bundler error parsing Gemfile: no implicit conversion of Array into String. Incorrect syntax at line 18: 'gem gem "faraday", "~> 2.9"'.
🔇 Additional comments (4)
lib/bas/utils/operaton/external_task_client.rb (4)
9-20: Excellent documentation and class structure.The class documentation is comprehensive and follows Ruby documentation conventions well. The example usage is particularly helpful for understanding the API.
21-33: Proper input validation and Faraday configuration.The initialization method now includes proper input validation as suggested in past reviews. The Faraday connection setup is correctly configured for JSON handling.
67-76: Robust topic parsing with good flexibility.The method handles both string and array inputs well, with proper trimming of whitespace. The implementation is clean and functional.
96-113: Comprehensive type mapping with proper Ruby type handling.The type conversion logic correctly maps Ruby types to the expected API types. The handling of boolean types using
TrueClassandFalseClassis particularly well done.
A client is implemented in Utils to allow bas interaction with the Operaton REST API via HTTP requests, allowing continuous polling of external service tasks, communication with it to execute a business logic and finally complete and resolve the external service task.
Summary by CodeRabbit