Skip to content

Conversation

@Aiden2014
Copy link
Collaborator

@Aiden2014 Aiden2014 commented Sep 15, 2025

koupleless/koupleless#405

Summary by CodeRabbit

  • New Features
    • Added --biz-model-version flag to deploy and undeploy commands to specify the business model version used during installation and uninstallation, including in Kubernetes environments.
  • Documentation
    • Updated command help and examples to show deployment and undeployment with a specified biz model version.
  • Chores
    • Bumped version to 0.2.5.
    • Minor formatting adjustment to a flag description (no behavior change).

@coderabbitai
Copy link

coderabbitai bot commented Sep 15, 2025

Walkthrough

Adds BizModelVersion support across deploy and undeploy: new CLI flags, propagation through parsing and payload construction, and inclusion in Ark BizModel JSON for install/uninstall (including K8s pod operations). Updates help/examples accordingly. Also bumps the exported Version constant from 0.2.3 to 0.2.5.

Changes

Cohort / File(s) Summary
CLI: Deploy with BizModelVersion
v1/cmd/deploy/deploy.go
Adds --biz-model-version flag; stores value; overrides parsed BizModelVersion in execParseBizModel when provided; includes BizModelVersion in uninstall/install payloads (local and K8s pod flows); updates help/example text; minor flag description formatting tweak.
CLI: Undeploy with BizModelVersion
v1/cmd/undeploy/undeploy.go
Adds --biz-model-version flag; passes BizModelVersion into Ark uninstall request via BizModel struct.
Ark BizModel type
v1/service/ark/types.go
Adds BizModelVersion string (json:"bizModelVersion,omitempty") field to BizModel struct.
Version bump
v1/constant/constant.go
Updates exported Version constant from "0.2.3" to "0.2.5".

Sequence Diagram(s)

sequenceDiagram
  autonumber
  participant U as User (CLI)
  participant DC as DeployCmd
  participant UC as UndeployCmd
  participant Ark as Ark Service (Local/K8s Pod)

  rect rgba(230,240,255,0.5)
    note over U,DC: Deploy flow with optional BizModelVersion
    U->>DC: deploy --biz-model-version V?
    DC->>DC: parse BizModel
    alt flag provided
      DC->>DC: override BizModel.BizModelVersion = V
    end
    DC->>Ark: uninstallBiz {bizModelVersion}
    Ark-->>DC: uninstall result
    DC->>Ark: installBiz {bizModelVersion}
    Ark-->>DC: install result
  end

  rect rgba(240,230,255,0.5)
    note over U,UC: Undeploy flow with BizModelVersion
    U->>UC: undeploy --biz-model-version V?
    UC->>Ark: uninstallBiz {bizModelVersion: V}
    Ark-->>UC: uninstall result
  end
Loading

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

Poem

I thump my paws—version hops to two-dot-five!
A model’s tale now carries “biz” alive.
Flags in the breeze, we ship with care,
Install, uninstall—versions everywhere.
In pods we warren, JSON tight,
Deploying dreams by moonlit byte. 🐇✨

Pre-merge checks and finishing touches

❌ Failed checks (2 warnings)
Check name Status Explanation Resolution
Title Check ⚠️ Warning The PR title states a fix for "模块 pod 替换导致 JVM 进程内模块实例消失" (module pod replacement causing JVM in-process module instances to disappear), but the changeset described in the raw summary adds a --biz-model-version CLI flag to deploy/undeploy, introduces BizModelVersion on ark.BizModel and propagates it in install/uninstall payloads, and bumps the Version constant; there are no edits touching pod-replacement or JVM lifecycle code, so the title is misleading and unrelated to the actual changes. Rename the PR to reflect the real changes (for example: "Add --biz-model-version flag and propagate BizModelVersion in Ark payloads; bump version to 0.2.5"), update the PR description to summarize the BizModelVersion intent and affected files, and if a JVM/pod-replacement bug is intended to be fixed here, include the corresponding code changes or split that work into a separate PR with tests and an explicit description.
Docstring Coverage ⚠️ Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. You can run @coderabbitai generate docstrings to improve docstring coverage.
✅ Passed checks (1 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
✨ Finishing touches
  • 📝 Generate Docstrings
🧪 Generate unit tests
  • Create PR with unit tests
  • Post copyable unit tests in a comment

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.

❤️ Share

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

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 (4)
v1/cmd/undeploy/undeploy.go (1)

135-136: Polish help text for clarity.

Minor grammar improvement.

- UnDeployCmd.Flags().StringVar(&bizModelVersionFlag, "biz-model-version", "", "the biz model version of ark container")
+ UnDeployCmd.Flags().StringVar(&bizModelVersionFlag, "biz-model-version", "", "biz model version used to identify the biz model in the ark container")
v1/cmd/deploy/deploy.go (3)

85-87: Fix typos in Scenario 4 example.

“witch” → “which”; capitalize Kubernetes; small phrasing tweak.

-Scenario 4: Build and deploy a bundle at current dir to a remote running ark container in k8s cluster with biz model version witch is pod key from kubernetes:
-	arkctl deploy --pod ${namespace}/${name} --biz-model-version ${your-biz-model-version}
+Scenario 4: Build and deploy a bundle at the current dir to a remote ark container in a K8s cluster with a biz model version (a pod key from Kubernetes):
+	arkctl deploy --pod ${namespace}/${name} --biz-model-version ${your-biz-model-version}

88-89: Grammar: “an maven” → “a Maven”; hyphenate multi‑module.

-Scenario 5: Build an maven multi module project and deploy a sub module to a running ark container:
+Scenario 5: Build a Maven multi‑module project and deploy a sub‑module to a running ark container:

469-472: Tighten flag help text.

Minor clarity/tense tweak.

-DeployCommand.Flags().StringVar(&bizModelVersionFlag, "biz-model-version", "", `
-If provided, arkctl will use this biz model version to deploy the biz module.
-`)
+DeployCommand.Flags().StringVar(&bizModelVersionFlag, "biz-model-version", "", `
+If provided, arkctl uses this biz model version during uninstall/install operations.
+`)
📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 5f2b906 and 128dafa.

📒 Files selected for processing (4)
  • v1/cmd/deploy/deploy.go (6 hunks)
  • v1/cmd/undeploy/undeploy.go (3 hunks)
  • v1/constant/constant.go (1 hunks)
  • v1/service/ark/types.go (1 hunks)
🔇 Additional comments (8)
v1/service/ark/types.go (1)

111-112: Add BizModelVersion to BizModel — LGTM; confirm server honors it.

Field/tag placement is fine and omitempty avoids noise. Please confirm Ark-side endpoints accept and use bizModelVersion.

If Ark server is in another repo, add a quick integration note or link in the PR description for traceability.

v1/cmd/undeploy/undeploy.go (2)

35-36: New flag wire-up — LGTM.

Flag variable introduction is straightforward and scoped correctly.


80-83: Propagate BizModelVersion in UnInstall payload — LGTM.

This keeps undeploy behavior consistent with deploy.

v1/cmd/deploy/deploy.go (4)

44-45: Flag added — LGTM.

Consistent naming with undeploy.


191-193: Override BizModelVersion from flag — LGTM.

Keeps CLI authoritative; safe with omitempty.


260-265: Include BizModelVersion in uninstall (K8s) payload — LGTM.

Matches server contract implied by types.


309-315: Include BizModelVersion in install (K8s) payload — LGTM.

Consistent with uninstall path.

v1/constant/constant.go (1)

21-21: Version bump to 0.2.5 — repo search clean; confirm release artifacts

v1/constant/constant.go:21 contains Version = "0.2.5"; no other pinned version literals or '0.2.3' occurrences found in repository. Approve changes; manually verify docs, release tags/changelog, and packaging/published artifacts reflect 0.2.5.

Copy link
Contributor

@Derrickers Derrickers left a comment

Choose a reason for hiding this comment

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

LGTM

@Derrickers Derrickers merged commit d548200 into koupleless:main Sep 18, 2025
1 check passed
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