- 
                Notifications
    You must be signed in to change notification settings 
- Fork 2
Add Bazel tooling #161
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Add Bazel tooling #161
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Really nice, thanks ! Just one thing about the build namespace
| # SPDX-FileCopyrightText: 2025-present Datadog, Inc. <dev@datadoghq.com> | ||
| # | ||
| # SPDX-License-Identifier: MIT | ||
| from __future__ import annotations | ||
|  | ||
| from typing import TYPE_CHECKING | ||
|  | ||
| import click | ||
|  | ||
| from dda.cli.base import dynamic_command, pass_app | ||
|  | ||
| if TYPE_CHECKING: | ||
| from dda.cli.application import Application | ||
|  | ||
|  | ||
| @dynamic_command( | ||
| short_help="Run Bazel commands", | ||
| context_settings={"help_option_names": [], "ignore_unknown_options": True}, | ||
| ) | ||
| @click.argument("args", nargs=-1) | ||
| @pass_app | ||
| def cmd(app: Application, *, args: tuple[str, ...]) -> None: | ||
| process = app.tools.bazel.attach(list(args), check=False) | ||
| app.abort(code=process.returncode) | 
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Just a thought for the future: do we want these kind of direct "tool-invocation" sort of commands to live in the build namespace ? It might be best to reserve that for the actual build tasks that we'll implement in the future, like dda build agent, dda build dogstatsd etc.
Maybe run or launch or exec or something similar would work ? We are after all just delegating the execution of some command to another tool
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
- I think bazelwould be the only exception for thebuildcommand group, it makes most sense for it to be there. The number of sister command groups would be smaller and more nested e.g.build compfor building individual components like the core Agent binary,build distfor building distributions like an MSI installer or a container, etc. The command could be instead something liketools bazel runbut then it wouldn't be as intuitive or discoverable.
- A nice part about having the command right under buildis that it removes the need to have an extra word (run, exec, etc.) and you can just pass arguments directly tobazel.
This adds a way to run Bazel commands and use a managed installation if there are no Bazel executables on PATH (or if configured as such).