-
Notifications
You must be signed in to change notification settings - Fork 20
Fix Python samples failing Azure deployment due to localhost binding #187
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
base: main
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -306,16 +306,20 @@ async def anonymous_claims(request, handler): | |
| if s.connect_ex(("127.0.0.1", desired_port)) == 0: | ||
| port = desired_port + 1 | ||
|
|
||
| # Detect production environment (Azure App Service sets WEBSITE_SITE_NAME) | ||
| is_production = os.getenv("WEBSITE_SITE_NAME") is not None | ||
| host = "0.0.0.0" if is_production else "localhost" | ||
|
|
||
| print("=" * 80) | ||
| print(f"🏢 {self.agent_class.__name__}") | ||
| print("=" * 80) | ||
| print(f"🔒 Auth: {'Enabled' if auth_configuration else 'Anonymous'}") | ||
| print(f"🚀 Server: localhost:{port}") | ||
| print(f"🚀 Server: {host}:{port}") | ||
| print(f"📚 Endpoint: http://localhost:{port}/api/messages") | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🟡 [CRM-001] Inconsistent Log Output The print statements for Endpoint and Health URLs still display Suggestion: Update to use |
||
| print(f"❤️ Health: http://localhost:{port}/api/health\n") | ||
|
Comment on lines
318
to
319
|
||
|
|
||
| try: | ||
| run_app(app, host="localhost", port=port, handle_signals=True) | ||
| run_app(app, host=host, port=port, handle_signals=True) | ||
| except KeyboardInterrupt: | ||
| print("\n👋 Server stopped") | ||
|
|
||
|
|
||
| Original file line number | Diff line number | Diff line change | ||||||||
|---|---|---|---|---|---|---|---|---|---|---|
|
|
@@ -434,6 +434,10 @@ async def anonymous_claims(request, handler): | |||||||||
| ) | ||||||||||
| port = desired_port + 1 | ||||||||||
|
|
||||||||||
| # Detect production environment (Azure App Service sets WEBSITE_SITE_NAME) | ||||||||||
| is_production = os.getenv("WEBSITE_SITE_NAME") is not None | ||||||||||
| host = "0.0.0.0" if is_production else "localhost" | ||||||||||
|
|
||||||||||
| print("=" * 80) | ||||||||||
| print(f"🏢 Generic Agent Host - {self.agent_class.__name__}") | ||||||||||
| print("=" * 80) | ||||||||||
|
|
@@ -442,13 +446,13 @@ async def anonymous_claims(request, handler): | |||||||||
| print("🎯 Compatible with Agents Playground") | ||||||||||
| if port != desired_port: | ||||||||||
| print(f"⚠️ Requested port {desired_port} busy; using fallback {port}") | ||||||||||
| print(f"\n🚀 Starting server on localhost:{port}") | ||||||||||
| print(f"\n🚀 Starting server on {host}:{port}") | ||||||||||
| print(f"📚 Bot Framework endpoint: http://localhost:{port}/api/messages") | ||||||||||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🟡 [CRM-002] Inconsistent Log Output Same issue as CRM-001 - endpoint and health URLs still reference Suggestion: Update for consistency with actual host binding. |
||||||||||
| print(f"❤️ Health: http://localhost:{port}/api/health") | ||||||||||
|
Comment on lines
450
to
451
|
||||||||||
| print(f"📚 Bot Framework endpoint: http://localhost:{port}/api/messages") | |
| print(f"❤️ Health: http://localhost:{port}/api/health") | |
| print(f"📚 Bot Framework endpoint: http://{host}:{port}/api/messages") | |
| print(f"❤️ Health: http://{host}:{port}/api/health") |
| Original file line number | Diff line number | Diff line change | ||||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
|
|
@@ -316,20 +316,24 @@ async def anonymous_claims(request, handler): | |||||||||||||||||
| ) | ||||||||||||||||||
| port = desired_port + 1 | ||||||||||||||||||
|
|
||||||||||||||||||
| # Detect production environment (Azure App Service sets WEBSITE_SITE_NAME) | ||||||||||||||||||
| is_production = os.getenv("WEBSITE_SITE_NAME") is not None | ||||||||||||||||||
| host = "0.0.0.0" if is_production else "localhost" | ||||||||||||||||||
|
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. We should decide if it's prod vs local based on ip address, not the otherway around, no? |
||||||||||||||||||
|
|
||||||||||||||||||
| print("=" * 80) | ||||||||||||||||||
| print(f"Generic Agent Host - {self.agent_class.__name__}") | ||||||||||||||||||
| print("=" * 80) | ||||||||||||||||||
| print(f"\nAuthentication: {'Enabled' if auth_configuration else 'Anonymous'}") | ||||||||||||||||||
| print("Using Microsoft Agents SDK patterns") | ||||||||||||||||||
| if port != desired_port: | ||||||||||||||||||
| print(f"Requested port {desired_port} busy; using fallback {port}") | ||||||||||||||||||
| print(f"\nStarting server on localhost:{port}") | ||||||||||||||||||
| print(f"\nStarting server on {host}:{port}") | ||||||||||||||||||
| print(f"Bot Framework endpoint: http://localhost:{port}/api/messages") | ||||||||||||||||||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🟡 [CRM-003] Inconsistent Log Output Same issue as CRM-001 - endpoint and health URLs still reference Suggestion: Update for consistency with actual host binding. |
||||||||||||||||||
| print(f"Health: http://localhost:{port}/api/health") | ||||||||||||||||||
|
Comment on lines
331
to
332
|
||||||||||||||||||
| print(f"Bot Framework endpoint: http://localhost:{port}/api/messages") | |
| print(f"Health: http://localhost:{port}/api/health") | |
| print(f"Bot Framework endpoint: http://{host}:{port}/api/messages") | |
| print(f"Health: http://{host}:{port}/api/health") | |
| if is_production: | |
| print( | |
| "Note: In production, replace '0.0.0.0' with the externally accessible host name or IP." | |
| ) |
| Original file line number | Diff line number | Diff line change | ||||||||
|---|---|---|---|---|---|---|---|---|---|---|
|
|
@@ -298,6 +298,10 @@ async def anonymous_claims(request, handler): | |||||||||
| ) | ||||||||||
| port = desired_port + 1 | ||||||||||
|
|
||||||||||
| # Detect production environment (Azure App Service sets WEBSITE_SITE_NAME) | ||||||||||
| is_production = os.getenv("WEBSITE_SITE_NAME") is not None | ||||||||||
| host = "0.0.0.0" if is_production else "localhost" | ||||||||||
|
|
||||||||||
| print("=" * 80) | ||||||||||
| print(f"🏢 Generic Agent Host - {self.agent_class.__name__}") | ||||||||||
| print("=" * 80) | ||||||||||
|
|
@@ -306,13 +310,13 @@ async def anonymous_claims(request, handler): | |||||||||
| print("🎯 Compatible with Agents Playground") | ||||||||||
| if port != desired_port: | ||||||||||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🟡 [CRM-004] Inconsistent Log Output Same issue as CRM-001 - endpoint and health URLs still reference Suggestion: Update for consistency with actual host binding. |
||||||||||
| print(f"⚠️ Requested port {desired_port} busy; using fallback {port}") | ||||||||||
| print(f"\n🚀 Starting server on localhost:{port}") | ||||||||||
| print(f"\n🚀 Starting server on {host}:{port}") | ||||||||||
| print(f"📚 Bot Framework endpoint: http://localhost:{port}/api/messages") | ||||||||||
| print(f"❤️ Health: http://localhost:{port}/api/health") | ||||||||||
|
Comment on lines
314
to
315
|
||||||||||
| print(f"📚 Bot Framework endpoint: http://localhost:{port}/api/messages") | |
| print(f"❤️ Health: http://localhost:{port}/api/health") | |
| print(f"📚 Bot Framework endpoint: http://{host}:{port}/api/messages") | |
| print(f"❤️ Health: http://{host}:{port}/api/health") |
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.
Why do we even need this variable? We should judge if the app is in prod vs dev based on environment variable like "ENVIRONMENT" or "ASP_ENVIRONMENT" or port number, not based on website name. I think this website name itself is causing a lot of confusion and we cannot set it prod vs development in E2E. Let me know if you think otherwise