[Service Bus] Update to use core-amqp #4794
Conversation
ramya0820
requested review from
danieljurek,
KarishmaGhiya,
mikeharder,
mitchdenny and
weshaggard
as code owners
ramya0820
requested review from
chradek,
michaeljqzq,
ramya-rao-a and
ShivangiReja
as code owners
ramya0820
force-pushed
the
sb-track-2-p2
branch
from
4f69aa6
to
8e436c5
Compare
|
We don't intend to update the master branch to use core-amqp just yet, therefore closing this PR. We first want to ship the ATOM based management apis being tracked in #1057 from the master branch without causing any breaking changes. Using core-amqp introduces breaking changes to Service Bus. |
|
Re-opening as the changes here build over |
ramya0820
changed the base branch from
master
to
feature/service-bus-track-2
ramya0820
changed the base branch from
feature/service-bus-track-2
to
master
ramya0820
changed the base branch from
master
to
feature/service-bus-track-2
ramya0820
changed the base branch from
feature/service-bus-track-2
to
master
|
The base branch can be updated to a different one from master to house the Track 2 related progress. @mikeharder can we create a new branch from the one submitted in this PR as the targeted Track 2 branch for Service Bus? Because this one correctly consolidates the changes required to match |
|
One thing we can do is to review the changes in PR basing off of master. And when ready to merge, can do so onto a new branch. Thoughts? @ramya-rao-a @chradek |
|
@ramya0820 Why can we not pull in the commits from the master into the existing branch Also, if we want to create a new branch with similar name, shouldn't we delete this existing branch?Having similar names branches creates a lot of unnecessary confusion |
ramya0820
changed the base branch from
master
to
feature/track-2-service-bus
|
/azp run js - client - ci |
|
Azure Pipelines could not run because the pipeline triggers exclude this branch/path. |
|
Update |
|
The branch |
|
@ramya0820 Please can you open a PR to the branch |
ramya0820
changed the base branch from
feature/track-2-service-bus
to
feature/service-bus-track-2
| @@ -58,7 +58,7 @@ | |||
| "unit-test": "npm run unit-test:node && npm run unit-test:browser" | |||
| }, | |||
| "dependencies": { | |||
| "@azure/event-hubs": "^2.1.1", | |||
| "@azure/event-hubs": "5.0.0-preview.3", | |||
There was a problem hiding this comment.
Why is this change being made? EPH will be using the older version of event hubs and is not compatible with version 5
| @@ -32,7 +32,7 @@ | |||
| "unit-test": "npm run unit-test:node && npm run unit-test:browser" | |||
| }, | |||
| "dependencies": { | |||
| "@azure/event-hubs": "^2.1.1", | |||
| "@azure/event-hubs": "5.0.0-preview.3", | |||
There was a problem hiding this comment.
Why is this change being made? The testhub package has not been updated to use the latest version of event hubs yet
There was a problem hiding this comment.
Reverted in #5324
It was required for a build fix, the common-version file changes from master fixes it
| @@ -45,13 +46,13 @@ export namespace ConnectionContext { | |||
|
|
|||
| export function create( | |||
| config: ConnectionConfig, | |||
| tokenProvider: TokenProvider, | |||
| tokenCredential: SharedKeyCredential | TokenCredential, | |||
There was a problem hiding this comment.
The connectionContext.ts file in the feature/service-bus-track-2 branch already has these changes, so I am not sure why these are appearing in the diffs as part of this PR.
There was a problem hiding this comment.
I believe the upstream branch was updated after the PR was created, it should have still pulled in the updates..
Will open a new PR
|
Something seems wrong when pulling from upstream branch on local. |
Makes Service Bus build fine with latest Track 2 core dependencies.
This would be precursor step to start making changes for Service Bus Track 2.