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

[EventHubs] Helper function to parse connection string #12684

Merged

Conversation

maorleger
Copy link
Member

@maorleger maorleger commented Nov 24, 2020

What

Add the following to the Event Hubs package:

  • A helper methods parseEventHubConnectionString that validates and
    parses a given connection string
  • An interface EventHubConnectionStringProperties that defines the
    output of the above method.

Why

In a recent PR the same change has been made for ServiceBus to provide a
parsing utility that can help transform a raw connection string for use
with credential-based client creation.

Callouts

  • This looks just about identical to the PR listed above. I'd love any feedback about
    whether I should leave it duplicated because of other work in that area or DRY up the code
    (and if so, where would be a good place to move the shared logic to)
  • I'm new to the codebase and TypeScript so please let me know if you'd like me to change anything

Implements #11894

@ghost ghost added Event Hubs customer-reported Issues that are reported by GitHub users external to the Azure organization. labels Nov 24, 2020
@ghost
Copy link

ghost commented Nov 24, 2020

Thank you for your contribution maorleger! We will review the pull request and get back to you soon.

@maorleger maorleger force-pushed the 11894-event-hubs-connectionstring-parser branch from 25e4035 to b14f329 Compare November 24, 2020 22:04
@HarshaNalluru
Copy link
Member

/azp run js - eventhubs - tests

@azure-pipelines
Copy link

No pipelines are associated with this pull request.

@HarshaNalluru
Copy link
Member

HarshaNalluru commented Nov 24, 2020

/azp run js - eventhubs-client - tests

@azure-pipelines
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@maorleger
Copy link
Member Author

/azp run js - eventhubs-client - tests

@azure-pipelines
Copy link

Commenter does not have sufficient privileges for PR 12684 in repo Azure/azure-sdk-for-js

@HarshaNalluru
Copy link
Member

/azp run js - eventhubs-client - tests

@azure-pipelines
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@ramya-rao-a ramya-rao-a removed the customer-reported Issues that are reported by GitHub users external to the Azure organization. label Jan 20, 2021
@ramya-rao-a
Copy link
Contributor

This looks just about identical to the PR listed above. I'd love any feedback about
whether I should leave it duplicated because of other work in that area or DRY up the code
(and if so, where would be a good place to move the shared logic to)

We have #12397 that tracks the need to have a folder holding common code between EH & SB, but is not shipped as a public package. Something like how we did with keyvault-common. We can then move this common code out.

@check-enforcer
Copy link

This pull request is protected by Check Enforcer.

What is Check Enforcer?

Check Enforcer helps ensure all pull requests are covered by at least one check-run (typically an Azure Pipeline). When all check-runs associated with this pull request pass then Check Enforcer itself will pass.

Why am I getting this message?

You are getting this message because Check Enforcer did not detect any check-runs being associated with this pull request within five minutes. This may indicate that your pull request is not covered by any pipelines and so Check Enforcer is correctly blocking the pull request being merged.

What should I do now?

If the check-enforcer check-run is not passing and all other check-runs associated with this PR are passing (excluding license-cla) then you could try telling Check Enforcer to evaluate your pull request again. You can do this by adding a comment to this pull request as follows:
/check-enforcer evaluate
Typically evaulation only takes a few seconds. If you know that your pull request is not covered by a pipeline and this is expected you can override Check Enforcer using the following command:
/check-enforcer override
Note that using the override command triggers alerts so that follow-up investigations can occur (PRs still need to be approved as normal).

What if I am onboarding a new service?

Often, new services do not have validation pipelines associated with them, in order to bootstrap pipelines for a new service, you can issue the following command as a pull request comment:
/azp run prepare-pipelines
This will run a pipeline that analyzes the source tree and creates the pipelines necessary to build and validate your pull request. Once the pipeline has been created you can trigger the pipeline using the following comment:
/azp run js - [service] - ci

@maorleger maorleger force-pushed the 11894-event-hubs-connectionstring-parser branch from e63b6f4 to e027090 Compare January 20, 2021 22:35
@maorleger maorleger force-pushed the 11894-event-hubs-connectionstring-parser branch from e027090 to c1d90f9 Compare January 22, 2021 18:10
maorleger and others added 8 commits January 27, 2021 10:46
This commit adds the following to the Event Hubs package to satisfy the
requirements listed in Azure#11894:

- A helper methods `parseEventHubConnectionString` that validates and
parses a given connection string
- An interface `EventHubConnectionStringProperties` that defines the
output of the above method.
Co-authored-by: Ramya Rao <ramya.rao.a@outlook.com>
@maorleger maorleger force-pushed the 11894-event-hubs-connectionstring-parser branch from c1d90f9 to 392192d Compare January 27, 2021 18:53
@maorleger
Copy link
Member Author

/azp run js - eventhubs-client - tests

@azure-pipelines
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@maorleger
Copy link
Member Author

maorleger commented Jan 28, 2021

Looks like this test has been failing since Sunday https://dev.azure.com/azure-sdk/internal/_build/results?buildId=699877&view=logs&j=38753cb9-b155-5978-2d3f-2d89d306e985&t=76836834-f1b7-5761-81f0-12284562de71

Took a look at it, ran it locally on my mac with the same node version and it passes in isolation so I suspect something must have changed during the private/public test split to cause some test pollution. Since it's unrelated to my changes I will make an issue to investigate this.

@maorleger
Copy link
Member Author

/check-enforcer override

@maorleger maorleger merged commit 06be1a1 into Azure:master Jan 28, 2021
@maorleger maorleger deleted the 11894-event-hubs-connectionstring-parser branch January 28, 2021 04:57
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants