Skip to content

Conversation

@leavesster
Copy link
Contributor

  • refactor: use three componet
  • refactor: rename field
  • test: update test case

@coderabbitai
Copy link

coderabbitai bot commented Sep 26, 2025

Note

Other AI code review bot(s) detected

CodeRabbit has detected other AI code review bot(s) in this pull request and will avoid duplicating their findings in the review comments. This may lead to a less comprehensive review.

Summary by CodeRabbit

  • 新功能
    • 支持三段式凭据占位符格式:${{OO_CREDENTIAL:type,name,id}},可提供名称与ID。
  • Bug 修复
    • 增强占位符早期校验,对缺少字段、逗号数量不符或前后缀错误的输入直接判为无效,避免异常行为。
  • 文档
    • 更新占位符格式与示例说明。
  • 提示
    • 旧格式 ${{OO_CREDENTIAL:type,id}} 将不再被解析。

Walkthrough

将凭证格式从两段式切换为三段式(type,name,id);CredentialInput 的字段由 value 重命名为 id;generate_credential_input 新增前后缀与逗号计数校验,解析为三部分并返回 type 与 id;相应单元测试同步更新。

Changes

Cohort / File(s) Summary of changes
Credential 解析与数据结构
executor/python_executor/credential.py
CredentialInput 构造器参数及字段 valueidgenerate_credential_input 文档与解析由两段改为三段(type,name,id),新增占位符前后缀与逗号计数的早期校验与失败时返回 None,解析时拆分为三部分并返回 CredentialInput(type, id)
测试更新
executor/tests/test_credential.py
测试改用三段式输入并把断言从旧的 value 字段改为 id;新增/修改用例以验证必须正好三段(两段格式应返回 None)及其它边界情况。

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~25 minutes

Possibly related PRs

  • feat: support credential input #433 — 修改相同模块(CredentialInputgenerate_credential_input),将字段与解析格式从两段→三段变更,具直接代码级关联。

Pre-merge checks

✅ Passed checks (2 passed)
Check name Status Explanation
Title Check ✅ Passed 标题使用了约定的类型前缀“refactor:”并简要说明了将主要逻辑改为采用三组件格式,符合本次更改的核心意图。
Description Check ✅ Passed PR 描述列出了使用三组件、字段重命名和测试更新等主要改动,与实际更改集保持一致,能够反映出本次变更的核心内容。

📜 Recent review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between ae7717f and 88b70fc.

📒 Files selected for processing (1)
  • executor/python_executor/credential.py (1 hunks)
🔇 Additional comments (1)
executor/python_executor/credential.py (1)

16-21: 请清理注释掉的日志占位符

这里的两条 # logger warning(...) 注释仍然保留着,与上一轮审查反馈一致,会让函数在关键路径上显得杂乱。建议要么接入真实的 logger.warning 调用,要么干脆删除注释。

-        # logger warning("Credential path format is incorrect. Expected to start with '${{OO_CREDENTIAL:' and end with '}}'.")
-        return None
-    
-    if credential_path.count(",") != 2:
-        # logger warning("Credential path format is incorrect. Expected exactly two commas.")
+        return None
+
+    if credential_path.count(",") != 2:
         return None

Comment @coderabbitai help to get the list of available commands and usage tips.

@leavesster leavesster requested a review from Copilot September 26, 2025 01:55
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull Request Overview

This pull request refactors the credential input handling to use a three-component format instead of two components, changing from type,id to type,name,id.

  • Updates the credential input format to include a name component between type and id
  • Renames the value field to id in the CredentialInput class
  • Adds validation for the three-component format and updates test cases accordingly

Reviewed Changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 3 comments.

File Description
executor/python_executor/credential.py Updates CredentialInput class to use id field and implements three-component parsing with proper validation
executor/tests/test_credential.py Updates test cases to use three-component format and validates the new id field instead of value

Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.

Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 2

🧹 Nitpick comments (1)
executor/python_executor/credential.py (1)

26-27: 避免未使用变量的静态检查告警

Line 26 展开出的 name 未被使用,Ruff 会报 RUF059。建议前缀下划线或直接忽略。

-            type, name, id = credential_path.split(",", maxsplit=2)
+            type, _name, id = credential_path.split(",", maxsplit=2)
📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 4e48d3f and 0a12e47.

📒 Files selected for processing (2)
  • executor/python_executor/credential.py (1 hunks)
  • executor/tests/test_credential.py (4 hunks)
🧰 Additional context used
🧬 Code graph analysis (1)
executor/tests/test_credential.py (1)
executor/python_executor/credential.py (2)
  • generate_credential_input (8-30)
  • CredentialInput (3-6)
🪛 Ruff (0.13.1)
executor/python_executor/credential.py

26-26: Unpacked variable name is never used

Prefix it with an underscore or any other dummy variable pattern

(RUF059)

⏰ 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). (2)
  • GitHub Check: check
  • GitHub Check: check

Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 0

🧹 Nitpick comments (1)
executor/python_executor/credential.py (1)

26-27: 请处理未使用的 name 变量

这里解包出的 name 从未被使用,会触发 Ruff 的 RUF059 告警,也增加了阅读时的噪点。直接将其前缀为下划线即可表明该值被有意忽略。

-            type, name, id = credential_path.split(",", maxsplit=2)
+            type, _name, id = credential_path.split(",", maxsplit=2)
📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 0a12e47 and ae7717f.

📒 Files selected for processing (1)
  • executor/python_executor/credential.py (1 hunks)
🧰 Additional context used
🪛 Ruff (0.13.1)
executor/python_executor/credential.py

26-26: Unpacked variable name is never used

Prefix it with an underscore or any other dummy variable pattern

(RUF059)

⏰ 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: check

@leavesster leavesster enabled auto-merge (squash) September 26, 2025 02:03
@leavesster leavesster disabled auto-merge September 26, 2025 02:03
@leavesster leavesster merged commit d21f47f into main Sep 26, 2025
8 checks passed
@leavesster leavesster deleted the use-three-path branch September 26, 2025 02:53
@oomol-bot oomol-bot mentioned this pull request Sep 26, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants