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

Hosting.Ngrok - support v2 configuration file with labels #447

Open
wants to merge 8 commits into
base: main
Choose a base branch
from

Conversation

programmation
Copy link

**Closes #446 **

Adds an extra field in NgrokEndpoint to store a dictionary of labels that will be applied to the endpoint in the configuration file.
Also supports output in v2 format. Although this is deprecated it appears to be the only documented way to apply labels to an ngrok endpoint.

PR Checklist

  • [x ] Created a feature/dev branch in your fork (vs. submitting directly from a commit on main)
  • [ x] Based off latest main branch of toolkit
  • [x ] PR doesn't include merge commits (always rebase on top of our main, if needed)
  • New integration
    • Docs are written
    • Added description of major feature to project description for NuGet package (4000 total character limit, so don't push entire description over that)
  • [ x] Tests for the changes have been added (for bug fixes / features) (if applicable)
  • [x ] Contains NO breaking changes
  • Every new API (including internal ones) has full XML docs
  • Code follows all style conventions

Other information

@aaronpowell
Copy link
Member

@programmation I've kicked off a workflow run. Assuming it builds there will be preview NuGet packages available that you can download (or pull from the Azure DevOps feed) to test it out.

@programmation
Copy link
Author

@dotnet-policy-service agree

@programmation
Copy link
Author

Apologies if this isn't the correct way to fix a (tiny) problem in the original PR.

@programmation programmation force-pushed the feature/ngrok-v2-configuration-file branch from db3cd20 to fe9f96e Compare February 5, 2025 23:30
@aaronpowell
Copy link
Member

I'll wait for @esskar to have a review as their the code owner of this integration

Copy link
Contributor

@esskar esskar left a comment

Choose a reason for hiding this comment

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

Looks good from the code side; thanks.

But edge labels/edge_id are no longer supported; there is no way to get an edge_id from ngrok anymore, instead you acquire a (free) custom domain from them - this was already supported by the integration.

@aaronpowell
Copy link
Member

Looks good from the code side; thanks.

But edge labels/edge_id are no longer supported; there is no way to get an edge_id from ngrok anymore, instead you acquire a (free) custom domain from them - this was already supported by the integration.

What issue might someone see as a result of edge labels not being supported and using the integration?

@programmation
Copy link
Author

The issue I had (and the reason for the PR) was that although I have a free URL, if I specify the URL in the Aspire config I get an error at ngrok, but if I omit it, ngrok creates a whole new randomised URL, which breaks my mobile test setup. I could not find any documentation at ngrok on how to upgrade from edge labels to whatever they're doing now so I reached out to them for clarification. In the meantime though I needed something to enable me to use the features that I've been using via docker-compose.

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.

Hosting.Ngrok support v2 configuration file with labels
3 participants