[Bug]: HttpPlugin overwrites server created by server_factory when specified

[kevinwoodward]

Bug Description

Introduced in this PR, when instantiating an HttpPlugin with a server_factory, self._server is overwritten in a check that is only intended to ensure that the server's port and the port on the class are aligned.

Steps to Reproduce

Run this code, observe that the custom server factory is called but the server instance custom_http_plugin._server has been overwritten by this else block.

In my case, I noticed this by attempting to specify proxy_headers=False without success.

import uvicorn
from fastapi import FastAPI
from microsoft_teams.apps.http_plugin import HttpPlugin


def custom_server_factory(app: FastAPI) -> uvicorn.Server:
    print('Creating custom server!')
    return uvicorn.Server(config=uvicorn.Config(app, host='0.0.0.0', port=80, proxy_headers=False))


custom_http_plugin = HttpPlugin(server_factory=custom_server_factory)


app = App(plugins=[custom_http_plugin])

Expected Behavior

The server created by the server_factory should be used and not overwritten.

Actual Behavior

The server created by the server_factory was overwritten.

SDK Version

2.0.0a9

Python Version

3.12.3

Additional Context

No response

[kevinwoodward]

I'll PR a fix shortly

[heyitsaamir]

Kevin Woodward (@kevinwoodward) I'd actually recommend using an http server adapter (see https://github.com/microsoft/teams.py/blob/main/examples/http-adapters/src/fastapi_non_managed.py) to accomplish this. We are planning on removing HttpPlugin in favor of this approach. This approach gives you full control over your server. Does this work for you?

[kevinwoodward]

@kevinwoodward I'd actually recommend using an http server adapter (see https://github.com/microsoft/teams.py/blob/main/examples/http-adapters/src/fastapi_non_managed.py) to accomplish this. We are planning on removing HttpPlugin in favor of this approach. This approach gives you full control over your server. Does this work for you?

The pattern works for me, but in that example it claims app.initialize() registers the /api/messages endpoint, that doesn't seem to be the case. Am I missing something?

[heyitsaamir]

@kevinwoodward I'd actually recommend using an http server adapter (see https://github.com/microsoft/teams.py/blob/main/examples/http-adapters/src/fastapi_non_managed.py) to accomplish this. We are planning on removing HttpPlugin in favor of this approach. This approach gives you full control over your server. Does this work for you?

The pattern works for me, but in that example it claims app.initialize() registers the /api/messages endpoint, that doesn't seem to be the case. Am I missing something?

No it's during construction. We will fix the comment.

PS: You can change the endpoint if you'd like too

[kevinwoodward]

@kevinwoodward I'd actually recommend using an http server adapter (see https://github.com/microsoft/teams.py/blob/main/examples/http-adapters/src/fastapi_non_managed.py) to accomplish this. We are planning on removing HttpPlugin in favor of this approach. This approach gives you full control over your server. Does this work for you?

The pattern works for me, but in that example it claims app.initialize() registers the /api/messages endpoint, that doesn't seem to be the case. Am I missing something?

No it's during construction. We will fix the comment.

PS: You can change the endpoint if you'd like too

Ah, I wasn't awaiting app.initialize()... 🤦

All good now