Feat/glab hosts #97
Feat/glab hosts #97
Conversation
📝 WalkthroughWalkthroughIntroduces an optional host parameter across deployment and provider APIs. WorkflowDeployer now stores host and passes it to provider.get_source_file_declaration. Providers (Github, Gitlab, Local) accept host; Gitlab derives host from non-gitlab.com URLs when not provided. The top-level deploy() signature includes host but may not pass it into WorkflowDeployer. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
actor User
participant CLI as CLI / deploy()
participant WD as WorkflowDeployer
participant Prov as Provider (Gitlab/Github/Local)
participant FS as Filesystem
User->>CLI: deploy(source_url, ..., host?)
CLI->>WD: new WorkflowDeployer(..., host?) %% If propagated
note over WD: Stores self.host
WD->>Prov: get_source_file_declaration(path, tag, branch, host=self.host)
alt Provider = Gitlab
note right of Prov: If host is None and URL not gitlab.com,<br/>derive host from URL
end
Prov-->>WD: source-file declaration string
WD->>FS: Write Snakefile with module declaration
FS-->>User: Snakefile created/updated
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Assessment against linked issues
Assessment against linked issues: Out-of-scope changes
Tip 🔌 Remote MCP (Model Context Protocol) integration is now available!Pro plan users can now connect to remote MCP servers from the Integrations page. Connect with popular remote MCPs such as Notion and Linear to add more context to your reviews and chats. ✨ Finishing Touches
🧪 Generate unit tests
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. CodeRabbit Commands (Invoked using PR/Issue comments)Type 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: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
snakedeploy/deploy.py (1)
280-283: Bug: host is not forwarded to WorkflowDeployer.deploy() accepts host but does not pass it to WorkflowDeployer, so self.host is always None. This breaks auto-injection for GitLab hosts when using deploy().
Apply this diff:
- with WorkflowDeployer( - source=source_url, dest=dest_path, tag=tag, branch=branch, force=force - ) as sd: + with WorkflowDeployer( + source=source_url, + dest=dest_path, + tag=tag, + branch=branch, + force=force, + host=host, + ) as sd: sd.deploy(name=name)
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (2)
snakedeploy/deploy.py(4 hunks)snakedeploy/providers.py(5 hunks)
🧰 Additional context used
📓 Path-based instructions (1)
**/*.py
⚙️ CodeRabbit configuration file
**/*.py: Do not try to improve formatting.
Do not suggest type annotations for functions that are defined inside of functions or methods.
Do not suggest type annotation of theselfargument of methods.
Do not suggest type annotation of theclsargument of classmethods.
Do not suggest return type annotation if a function or method does not contain areturnstatement.
Files:
snakedeploy/deploy.pysnakedeploy/providers.py
🧬 Code graph analysis (1)
snakedeploy/providers.py (1)
snakedeploy/exceptions.py (1)
UserError(43-44)
🔇 Additional comments (8)
snakedeploy/providers.py (4)
1-8: Imports update looks good.Optional was added and imports reorganized appropriately.
36-38: Abstract method signatures: OK.Adding explicit annotations and placeholders is fine.
Also applies to: 40-42, 44-46, 48-50
82-84: Local.get_source_file_declaration: OK (host intentionally unused).Signature compatibility maintained; ignoring host for local sources is reasonable.
116-118: Confirm intent: host is ignored for Github provider.If GitHub Enterprise support is desired, consider propagating host here (only if Snakemake’s github() accepts it); otherwise, leaving it unused is fine for this PR’s GitLab focus.
snakedeploy/deploy.py (4)
24-25: Add host to WorkflowDeployer: OK.Constructor signature change looks good.
33-33: Store host on instance: OK.Passed through for later rendering.
231-233: Pass host to provider declaration: OK.Host is now threaded into the Snakefile template call.
258-259: Top-level deploy() accepts host: OK.Public API extended as per PR objective.
| def get_source_file_declaration( | ||
| self, path: str, tag: str, branch: str, host: Optional[str] = None | ||
| ): | ||
| owner_repo = "/".join(self.source_url.split("/")[-2:]) | ||
| url_host = self.source_url.split("/")[2] | ||
| if host is None and url_host != "gitlab.com": | ||
| host = url_host | ||
| if not (tag or branch): | ||
| raise UserError("Either tag or branch has to be specified for deployment.") | ||
| ref_arg = f'tag="{tag}"' if tag is not None else f'branch="{branch}"' | ||
| host_arg = f'host="{host}"' if host is not None else "" | ||
| return f'{self.name}("{owner_repo}", path="{path}", {ref_arg}, {host_arg})' | ||
|
|
There was a problem hiding this comment.
🛠️ Refactor suggestion
Fix GitLab namespace parsing and avoid dangling comma in call.
- owner_repo drops parent groups for nested namespaces (e.g., group/subgroup/repo).
- Building the call with an empty host_arg yields a trailing comma in the argument list.
Proposed fix:
- Use the full path after the host for owner_repo.
- Build args dynamically to avoid the dangling comma.
Apply this diff:
def get_source_file_declaration(
self, path: str, tag: str, branch: str, host: Optional[str] = None
):
- owner_repo = "/".join(self.source_url.split("/")[-2:])
- url_host = self.source_url.split("/")[2]
+ # Use full namespace path (supports nested groups)
+ owner_repo = "/".join(self.source_url.split("/")[3:])
+ url_host = self.source_url.split("/")[2]
if host is None and url_host != "gitlab.com":
host = url_host
if not (tag or branch):
raise UserError("Either tag or branch has to be specified for deployment.")
ref_arg = f'tag="{tag}"' if tag is not None else f'branch="{branch}"'
- host_arg = f'host="{host}"' if host is not None else ""
- return f'{self.name}("{owner_repo}", path="{path}", {ref_arg}, {host_arg})'
+ args = [f'"{owner_repo}"', f'path="{path}"', ref_arg]
+ if host is not None:
+ args.append(f'host="{host}"')
+ return f'{self.name}({", ".join(args)})'📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| def get_source_file_declaration( | |
| self, path: str, tag: str, branch: str, host: Optional[str] = None | |
| ): | |
| owner_repo = "/".join(self.source_url.split("/")[-2:]) | |
| url_host = self.source_url.split("/")[2] | |
| if host is None and url_host != "gitlab.com": | |
| host = url_host | |
| if not (tag or branch): | |
| raise UserError("Either tag or branch has to be specified for deployment.") | |
| ref_arg = f'tag="{tag}"' if tag is not None else f'branch="{branch}"' | |
| host_arg = f'host="{host}"' if host is not None else "" | |
| return f'{self.name}("{owner_repo}", path="{path}", {ref_arg}, {host_arg})' | |
| def get_source_file_declaration( | |
| self, path: str, tag: str, branch: str, host: Optional[str] = None | |
| ): | |
| # Use full namespace path (supports nested groups) | |
| owner_repo = "/".join(self.source_url.split("/")[3:]) | |
| url_host = self.source_url.split("/")[2] | |
| if host is None and url_host != "gitlab.com": | |
| host = url_host | |
| if not (tag or branch): | |
| raise UserError("Either tag or branch has to be specified for deployment.") | |
| ref_arg = f'tag="{tag}"' if tag is not None else f'branch="{branch}"' | |
| - host_arg = f'host="{host}"' if host is not None else "" | |
| args = [f'"{owner_repo}"', f'path="{path}"', ref_arg] | |
| if host is not None: | |
| args.append(f'host="{host}"') | |
| return f'{self.name}({", ".join(args)})' |
🤖 Prompt for AI Agents
In snakedeploy/providers.py around lines 130 to 142, owner_repo currently uses
only the last two path segments which loses parent groups for nested GitLab
namespaces and host handling can produce an empty host_arg that leaves a
dangling comma in the returned call string; to fix, derive owner_repo as the
full path portion after the host (i.e., join all segments after the domain),
determine url_host from the netloc/second segment as you already do, build a
list of argument strings starting with the required '"{owner_repo}"' and
'path="{path}"', add the ref argument as tag or branch depending on which is
set, and only append a host="..." argument if host is not None, then return the
call by joining the args with ", " so no trailing comma appears.
1 participant
closes #96
This automatically detects the gitlab host you used and injects it into the provided snakemake module.
Summary by CodeRabbit