Skip to content

Conversation

@maorleger
Copy link
Member

@maorleger maorleger commented Jun 23, 2021

What

  • Update core-http to 2.0.0
  • Update core-lro to 2.0.0
  • Update packages to use latest version

Why

This will support our last breaking change for core-tracing, and allow everyone to be on the same minimum version. This will also allow us to target ES2017 across the board.

@hectorhdzg
Copy link
Member

@maorleger can you add more details about what was the breaking change in core-tracing?, Azure Monitor Exporter is directly referencing OpenTelemetry packages and I want to be sure there will not be conflicts in there

@ramya-rao-a
Copy link
Contributor

@maorleger In #15835, we talk about a different version bump for core-http. Should we perhaps do that first?

@maorleger
Copy link
Member Author

maorleger commented Jun 23, 2021

@hectorhdzg sure! there are a few changes that matter:

  • Span#context is now Span#spanContext
  • Link#context was a subset of SpanContext, but is now a full context.

My understanding is that we want to mirror OTel's interfaces so those changes carried over to us.
Those are handled in the core libraries that use these APIs - specifically the first change which we directly reference in our core packages.

@ramya-rao-a - good question. I don't know what we should do here - if it's all the same to you I'd rather defer to you @chradek @jeremymeng or @xirzec on this question

@maorleger
Copy link
Member Author

@ramya-rao-a I chatted with @xirzec offline and we can bump the major to 2.0 for core-http

@deyaaeldeen do you think you can land #15835 before the code freeze? Since we're already bumping the major version this would be the time to get that in

@deyaaeldeen
Copy link
Member

@maorleger I will take a look now.

@maorleger maorleger changed the title [core] - Set core-http 1.2.7 as the minimum version [core] - Bump core-http to 2.0.0 and update packages that depend on it Jun 23, 2021
@deyaaeldeen
Copy link
Member

@maorleger I believe #15835 is ready now to get merged soon.

@maorleger
Copy link
Member Author

@EmmaZhu fyi - could I get storage folks to approve as well?

@jeremymeng jeremymeng added the Client This issue points to a problem in the data-plane of the library. label Jun 24, 2021
@maorleger maorleger force-pushed the everyone-on-core-http-127 branch from a47ebbc to 4216c79 Compare June 24, 2021 15:09
@maorleger maorleger changed the title [core] - Bump core-http to 2.0.0 and update packages that depend on it [core] - Bump core-http to 2.0.0 and core-lro to 2.0.0 Jun 24, 2021
@maorleger maorleger force-pushed the everyone-on-core-http-127 branch from 4216c79 to 01ea003 Compare June 24, 2021 17:55
@maorleger
Copy link
Member Author

Known issue in test-utils which should be fixed by #15899 - the tests are all passing just the copy package step fails

Copy link
Member

@deyaaeldeen deyaaeldeen left a comment

Choose a reason for hiding this comment

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

:shipit:

@maorleger
Copy link
Member Author

maorleger commented Jun 24, 2021

/check-enforcer override

Edit: overriding because the only error is the known and unrelated test-utils failure which will be fixed by #15899 - the tests all pass

@maorleger maorleger merged commit 5903b7c into Azure:main Jun 24, 2021
@maorleger maorleger deleted the everyone-on-core-http-127 branch June 24, 2021 19:59
maorleger added a commit that referenced this pull request Jul 7, 2021
…blob (#16182)

We recently discovered an issue caused by bumping core-http by a major version (#15925) where if a transitive dependency
is still allowing an older version we would end up with two versions of core-http / core-lro.

This is a problem for two reasons:
- bundle size
- incompatibility of enums between two versions

This was discovered via failed nightly tests and while some of this was fixed by #16180, we still need to audit all the packages and ensure that:

For every package that moved to [email protected], all other dependencies of that package are pinned to a min-version that also uses [email protected]

Luckily that is a very small number of packages - mainly communication (fixed) and storage (this PR)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

App Configuration Azure.ApplicationModel.Configuration Azure.Core Azure.Identity Client This issue points to a problem in the data-plane of the library. Cognitive - Form Recognizer Cognitive - Metrics Advisor Communication Digital Twins EngSys This issue is impacting the engineering system. KeyVault Monitor Monitor, Monitor Ingestion, Monitor Query Search Service Bus Storage Storage Service (Queues, Blobs, Files) Synapse

Projects

None yet

Development

Successfully merging this pull request may close these issues.

8 participants