Skip to content

Improve Azure example configuration handling #430

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.

Sign up for GitHub

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

Open
guming3d wants to merge 1 commit into
base: main
Choose a base branch
from
+31 −4
Open

Improve Azure example configuration handling #430

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
11 changes: 10 additions & 1 deletion examples/azure/aoai_finetune.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@

from agentlightning.adapter.messages import OpenAIMessages, TraceToMessages
from agentlightning.algorithm import Algorithm
from agentlightning.algorithm.utils import batch_iter_over_dataset
from agentlightning.algorithm.apo.apo import batch_iter_over_dataset
Copy link

Copilot AI Dec 23, 2025

Choose a reason for hiding this comment

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

The import path change is incorrect. The function batch_iter_over_dataset is defined in agentlightning.algorithm.utils (not in agentlightning.algorithm.apo.apo). While apo.py imports it from utils, importing directly from apo.py creates an indirect dependency. Please import from the original location: from agentlightning.algorithm.utils import batch_iter_over_dataset.

Suggested change
from agentlightning.algorithm.apo.apo import batch_iter_over_dataset
from agentlightning.algorithm.utils import batch_iter_over_dataset

Copilot uses AI. Check for mistakes.
from agentlightning.reward import find_final_reward
from agentlightning.types import LLM, RolloutMode, TaskInput

Expand All @@ -29,6 +29,11 @@
FINETUNE_JOB_POLL_INTERVAL = 60


from dotenv import load_dotenv

# Load environment variables from .env file (override existing)
load_dotenv(override=True)

class AzureOpenAIFinetune(Algorithm):
"""Coordinate iterative fine-tuning runs for an Azure OpenAI deployment.

Expand Down Expand Up @@ -176,6 +181,10 @@ async def run( # type: ignore
training_data = await self.prepare_data_for_training(messages_group, reward_group, "train")
self._log_info(f"[Stage 4] Prepared {len(training_data)} training examples after filtering.")

self.openai_client = OpenAI(
Copy link
Contributor

@ultmaster ultmaster Dec 23, 2025

Choose a reason for hiding this comment

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

Maybe we should encourage users to use the correct endpoint ending with /openai/v1. In other words, the documentation should be updated instead of the code.

api_key=self.azure_openai_api_key,
base_url=self.azure_openai_endpoint + "/openai/v1/",
)
Comment on lines +184 to +187
Copy link

Copilot AI Dec 23, 2025

Choose a reason for hiding this comment

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

The OpenAI client is being re-initialized inside the run loop, which overrides the client already created in __init__ at lines 134-137. This re-initialization changes the base_url by appending "/openai/v1/" to the endpoint. If this path modification is necessary for the fine-tuning API, it should be done in __init__ instead to avoid confusion and ensure consistency. If both URLs are needed for different purposes, consider using separate client instances with descriptive names.

Suggested change
self.openai_client = OpenAI(
api_key=self.azure_openai_api_key,
base_url=self.azure_openai_endpoint + "/openai/v1/",
)

Copilot uses AI. Check for mistakes.
# (5) Perform fine-tuning
self._log_info(f"[Stage 5] Starting fine-tuning for model {training_model_name}...")
training_model_name = self.finetune(training_data, training_model_name, i_iteration)
Expand Down
18 changes: 17 additions & 1 deletion examples/azure/capital_agent.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,13 @@

from agentlightning import LLM, AgentOpsTracer, InMemoryLightningStore, LitAgentRunner, rollout

from dotenv import load_dotenv

# Load environment variables from .env file (override existing)
load_dotenv(override=True)
Copy link
Contributor

@ultmaster ultmaster Dec 23, 2025

Choose a reason for hiding this comment

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

dotenv run is a better idea than load_dotenv.




Copy link

Copilot AI Dec 23, 2025

Choose a reason for hiding this comment

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

There are extra blank lines after the dotenv loading code. According to PEP 8 style guidelines, there should only be two blank lines between top-level code blocks. Remove one of the blank lines to maintain consistency.

Suggested change

Copilot uses AI. Check for mistakes.
CAPITALS = {
"japan": "Tokyo",
"france": "Paris",
Expand Down Expand Up @@ -84,7 +91,16 @@ def capital_agent(task: CapitalTask, llm: LLM) -> float:
prompt = task["input"]
expected = task["output"]

openai_client = openai.OpenAI(base_url=llm.endpoint, api_key=os.getenv("AZURE_OPENAI_API_KEY", ""))
api_version = os.getenv("AZURE_OPENAI_API_VERSION", "")
if not api_version:
raise ValueError("Set AZURE_OPENAI_API_VERSION to match your Azure OpenAI resource.")
Copy link

Copilot AI Dec 23, 2025

Choose a reason for hiding this comment

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

The error message states "Set AZURE_OPENAI_API_VERSION to match your Azure OpenAI resource" but doesn't provide guidance on what a valid value looks like. Consider enhancing the error message to include an example of a valid API version format (e.g., "2024-08-01-preview") to help users configure this correctly.

Suggested change
raise ValueError("Set AZURE_OPENAI_API_VERSION to match your Azure OpenAI resource.")
raise ValueError(
"Set AZURE_OPENAI_API_VERSION (for example, '2024-08-01-preview') to match your Azure OpenAI resource's API version."
)

Copilot uses AI. Check for mistakes.

# Use AzureOpenAI client for Azure endpoints
openai_client = openai.AzureOpenAI(
Copy link
Contributor

@ultmaster ultmaster Dec 23, 2025

Choose a reason for hiding this comment

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

Why AzureOpenAI is a must here?

azure_endpoint=llm.endpoint,
api_key=os.getenv("AZURE_OPENAI_API_KEY", ""),
api_version=api_version,
)

messages: List[ChatCompletionMessageParam] = [
{"role": "system", "content": SYSTEM},
Expand Down
6 changes: 4 additions & 2 deletions examples/azure/train_capital_agent.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,9 @@
from capital_agent import capital_agent
from rich.console import Console

from agentlightning import TraceToMessages, Trainer, setup_logging
from agentlightning.adapter.messages import TraceToMessages
from agentlightning.trainer.trainer import Trainer
from agentlightning.logging import configure_logger

console = Console()

Expand All @@ -20,7 +22,7 @@ def parse_args() -> argparse.Namespace:


def main():
setup_logging()
configure_logger()
Copy link

Copilot AI Dec 23, 2025

Choose a reason for hiding this comment

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

The function configure_logger() is deprecated in favor of setup_logging(). According to the documentation in agentlightning/logging.py, this function emits a deprecation warning. Consider using setup_logging() instead, which is imported as from agentlightning import setup_logging (or use the setup function from agentlightning.logging).

Copilot uses AI. Check for mistakes.
args = parse_args()
finetune_algo = AzureOpenAIFinetune(
base_deployment_name="gpt-4.1-mini",
Expand Down
Loading