Skip to content
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

Using PORT as env in workbench app #168

Merged
merged 2 commits into from
Oct 25, 2024
Merged

Conversation

tommasodotNET
Copy link
Contributor

This PR addresses #167

Update server configuration to use 'localhost' as the host and dynamically assign the port number

Copy link
Contributor

@bkrabach bkrabach left a comment

Choose a reason for hiding this comment

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

We have app registrations and other configurations dependent upon the explicit use of 127.0.0.1 along with the specific port and https requirement. We will need to investigate the impact of the host change in the places we're using it.

As alternative or compromise until we investigate further, perhaps we apply the same approach you have provided for port and allow the host to be configured via env var, with the default remaining at 127.0.0.1?

@tommasodotNET
Copy link
Contributor Author

Using localhost won't break the app registration. I've tested and the login still works fine...

@bkrabach
Copy link
Contributor

bkrabach commented Oct 25, 2024

Using localhost won't break the app registration. I've tested and the login still works fine...

Appreciate the testing and verification of that! Have you tried it with Codespaces as well? This is the primary path we've suggested for folks who want to get a quick dev environment to try it out and hack on their own assistants.

We have a hosted version that we use for our team and others that has a different app registration with some customizations that we cannot modify under current Azure portal changes and some other places where we're explicit about using https://127.0.0.1:4000 that is some work to change, beyond what is visible in the repo. Those are the things I need to check on before committing a change to the default behavior.

Copy link
Contributor

@bkrabach bkrabach left a comment

Choose a reason for hiding this comment

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

LGTM, thanks!

@bkrabach bkrabach merged commit 8e0bbf0 into microsoft:main Oct 25, 2024
4 checks passed
@tommasodotNET tommasodotNET deleted the #167 branch October 25, 2024 18:41
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants