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.

Sign up for GitHub

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

Jump to bottom

Downgrade several 7.0.* packages to 6.0.* #1799

Merged
merged 5 commits into from
Jul 4, 2023
Merged

Downgrade several 7.0.* packages to 6.0.* #1799

merged 5 commits into from
Jul 4, 2023

Conversation

stephentoub
Copy link
Member

@stephentoub stephentoub commented Jun 30, 2023 •
edited
Loading

This pains me, as it's a technical step backwards. But there are two problems today:

  1. The .NET support policy today unfortunately applies not only to the .NET SDK and runtime but also to nuget packages released in the same wave, which means that if someone is relying on eg System.Text.Json and is running on .NET 6, having SK depend on System.Text.Json with a minimum version of 7.0 forces the consumer to also use the 7.0 version of System.Text.Json from the nuget, which then means that one assembly falls under the STS support policy instead of LTS support policy, which means that one assembly will have support end for it a few months earlier than it otherwise would. I've raised this issue for further evaluation on the .NET side of things, but in the meantime, this lowers the version number to remove the perceived problem and possible barrier to adoption.
  2. Azure Functions has two deployment models: in-process and isolated. Ideally functions use isolated, which gives them the freedom to reference whatever they need. The in-process model is exactly what it sounds like: the function runs in the same process as the host, and the host pins several dependencies at 6.0 versions. That means that if a function references a 7.0 version, it'll fail to load in the in-process model. While we'd like for folks to be using the isolated model, we can't force it, and we don't want a need for the in-process model to block SK usage.

This commit downgrades back to the 6.0 versions, at least where possible. Some of the connectors reference libraries (e.g. NRedisStack, pgvector, etc.) that themselves have a 7.0 dependency.

Closes #1793

@stephentoub
Downgrade several 7.0.* packages to 6.0.*
8633a57 
This pains me, as it's a technical step backwards.  But there are two problems today:
1) The .NET support policy today unfortunately applies not only to the .NET SDK and runtime but also to nuget packages released in the same wave, which means that if someone is relying on eg System.Text.Json and is running on .NET 6, having SK depend on System.Text.Json with a minimum version of 7.0 forces the consumer to also use the 7.0 version of System.Text.Json from the nuget, which then means that one assembly falls under the STS support policy instead of LTS support policy, which means that one assembly will have support end for it a few months earlier than it otherwise would.  I've raised this issue for further evaluation on the .NET side of things, but in the meantime, this lowers the version number to remove the perceived problem and possible barrier to adoption.
2) Azure Functions has two deployment models: in-process and isolated.  Ideally functions use isolated, which gives them the freedom to reference whatever they need.  The in-process model is exactly what it sounds like: the function runs in the same process as the host, and the host pins several dependencies at 6.0 versions.  That means that if a function references a 7.0 version, it'll fail to load in the in-process model.  While we'd like for folks to be using the isolated model, we can't force it, and we don't want a need for the in-process model to block SK usage.

This commit downgrades back to the 6.0 versions, at least where possible.  Some of the connectors reference libraries (e.g. NRedisStack, pgvector, etc.) that themselves have a 7.0 dependency.
@stephentoub stephentoub requested a review from a team as a code owner June 30, 2023 20:33
@github-actions github-actions bot added .NET Issue or Pull requests regarding .NET code kernel Issues or pull requests impacting the core kernel labels Jun 30, 2023
@stephentoub
Copy link
Member Author

stephentoub commented Jun 30, 2023 •
edited
Loading

cc: @ericstj, @shawncal

@shawncal
Copy link
Member

shawncal commented Jul 3, 2023

Could you please also add the downgraded packages to ignore sections in https://github.com/microsoft/semantic-kernel/blob/main/.github/dependabot.yml?
Otherwise, we'll be right back where we are in a couple days ;p

@stephentoub
Copy link
Member Author

Could you please also add the downgraded packages to ignore sections

I tried. Let me know if this looks right / wrong.

@shawncal shawncal added this pull request to the merge queue Jul 4, 2023
Merged via the queue into microsoft:main with commit e7ba201 Jul 4, 2023
10 checks passed
shawncal added a commit to shawncal/semantic-kernel that referenced this pull request Jul 6, 2023
@stephentoub @shawncal
Downgrade several 7.0.* packages to 6.0.* (microsoft#1799)
fa65843 
This pains me, as it's a technical step backwards. But there are two
problems today:
1) The .NET support policy today unfortunately applies not only to the
.NET SDK and runtime but also to nuget packages released in the same
wave, which means that if someone is relying on eg System.Text.Json and
is running on .NET 6, having SK depend on System.Text.Json with a
minimum version of 7.0 forces the consumer to also use the 7.0 version
of System.Text.Json from the nuget, which then means that one assembly
falls under the STS support policy instead of LTS support policy, which
means that one assembly will have support end for it a few months
earlier than it otherwise would. I've raised this issue for further
evaluation on the .NET side of things, but in the meantime, this lowers
the version number to remove the perceived problem and possible barrier
to adoption.
2) Azure Functions has two deployment models: in-process and isolated.
Ideally functions use isolated, which gives them the freedom to
reference whatever they need. The in-process model is exactly what it
sounds like: the function runs in the same process as the host, and the
host pins several dependencies at 6.0 versions. That means that if a
function references a 7.0 version, it'll fail to load in the in-process
model. While we'd like for folks to be using the isolated model, we
can't force it, and we don't want a need for the in-process model to
block SK usage.

This commit downgrades back to the 6.0 versions, at least where
possible. Some of the connectors reference libraries (e.g. NRedisStack,
pgvector, etc.) that themselves have a 7.0 dependency.

Closes microsoft#1793

---------

Co-authored-by: Shawn Callegari <36091529+shawncal@users.noreply.github.com>
This was referenced Jul 7, 2023
Merged
Closed
@kou kou mentioned this pull request Jul 9, 2023
Closed
@Woland2k
Copy link
Contributor

Thank you for the update and listening for the community on this!

@dmytrostruk dmytrostruk mentioned this pull request Sep 7, 2023
Merged
4 tasks
github-merge-queue bot pushed a commit that referenced this pull request Sep 7, 2023
@dmytrostruk
.Net: Downgrade System.Diagnostics.DiagnosticSource package (#2746)
574c8ae 
### Motivation and Context

<!-- Thank you for your contribution to the semantic-kernel repo!
Please help reviewers and future users, providing the following
information:
  1. Why is this change required?
  2. What problem does it solve?
  3. What scenario does it contribute to?
  4. If it fixes an open issue, please link to the issue here.
-->

Resolves: #2646

As described in PR
#1799, we should keep
.NET packages at version 6.0.*, to support package compatibility and
allow to use Azure Functions in-process deployment model.

### Description

<!-- Describe your changes, the overall approach, the underlying design.
These notes will help understanding how your code works. Thanks! -->
1. Downgraded `System.Diagnostics.DiagnosticSource` package version from
7.0.2 to 6.0.0.
2. Manually tested telemetry functionality and verified that metering
logic works as expected.

### Contribution Checklist

<!-- Before submitting this PR, please make sure: -->

- [x] The code builds clean without any errors or warnings
- [x] The PR follows the [SK Contribution
Guidelines](https://github.com/microsoft/semantic-kernel/blob/main/CONTRIBUTING.md)
and the [pre-submission formatting
script](https://github.com/microsoft/semantic-kernel/blob/main/CONTRIBUTING.md#development-scripts)
raises no violations
- [x] All unit tests pass, and I have added new tests where possible
- [x] I didn't break anyone 😄
@JadynWong JadynWong mentioned this pull request Sep 26, 2023
Merged
4 tasks
github-merge-queue bot pushed a commit that referenced this pull request Sep 29, 2023
@JadynWong @dmytrostruk
.Net: Fix redis package reference (#2985)
8c07296 
### Motivation and Context
Fix #2953

### Description
Looks like, since #1799, there's been a problem. However, due to
references from other projects, tests and samples have not shown the
problem.

### Contribution Checklist

<!-- Before submitting this PR, please make sure: -->

- [x] The code builds clean without any errors or warnings
- [x] The PR follows the [SK Contribution
Guidelines](https://github.com/microsoft/semantic-kernel/blob/main/CONTRIBUTING.md)
and the [pre-submission formatting
script](https://github.com/microsoft/semantic-kernel/blob/main/CONTRIBUTING.md#development-scripts)
raises no violations
- [x] All unit tests pass, and I have added new tests where possible
- [x] I didn't break anyone 😄

Co-authored-by: Dmytro Struk <13853051+dmytrostruk@users.noreply.github.com>
SOE-YoungS pushed a commit to SOE-YoungS/semantic-kernel that referenced this pull request Nov 1, 2023
@dmytrostruk @SOE-YoungS
.Net: Downgrade System.Diagnostics.DiagnosticSource package (microsof…
54d42cf 
…t#2746)

### Motivation and Context

<!-- Thank you for your contribution to the semantic-kernel repo!
Please help reviewers and future users, providing the following
information:
  1. Why is this change required?
  2. What problem does it solve?
  3. What scenario does it contribute to?
  4. If it fixes an open issue, please link to the issue here.
-->

Resolves: microsoft#2646

As described in PR
microsoft#1799, we should keep
.NET packages at version 6.0.*, to support package compatibility and
allow to use Azure Functions in-process deployment model.

### Description

<!-- Describe your changes, the overall approach, the underlying design.
These notes will help understanding how your code works. Thanks! -->
1. Downgraded `System.Diagnostics.DiagnosticSource` package version from
7.0.2 to 6.0.0.
2. Manually tested telemetry functionality and verified that metering
logic works as expected.

### Contribution Checklist

<!-- Before submitting this PR, please make sure: -->

- [x] The code builds clean without any errors or warnings
- [x] The PR follows the [SK Contribution
Guidelines](https://github.com/microsoft/semantic-kernel/blob/main/CONTRIBUTING.md)
and the [pre-submission formatting
script](https://github.com/microsoft/semantic-kernel/blob/main/CONTRIBUTING.md#development-scripts)
raises no violations
- [x] All unit tests pass, and I have added new tests where possible
- [x] I didn't break anyone 😄
SOE-YoungS pushed a commit to SOE-YoungS/semantic-kernel that referenced this pull request Nov 1, 2023
@JadynWong @dmytrostruk @SOE-YoungS
.Net: Fix redis package reference (microsoft#2985)
862c712 
### Motivation and Context
Fix microsoft#2953

### Description
Looks like, since microsoft#1799, there's been a problem. However, due to
references from other projects, tests and samples have not shown the
problem.

### Contribution Checklist

<!-- Before submitting this PR, please make sure: -->

- [x] The code builds clean without any errors or warnings
- [x] The PR follows the [SK Contribution
Guidelines](https://github.com/microsoft/semantic-kernel/blob/main/CONTRIBUTING.md)
and the [pre-submission formatting
script](https://github.com/microsoft/semantic-kernel/blob/main/CONTRIBUTING.md#development-scripts)
raises no violations
- [x] All unit tests pass, and I have added new tests where possible
- [x] I didn't break anyone 😄

Co-authored-by: Dmytro Struk <13853051+dmytrostruk@users.noreply.github.com>
johnoliver pushed a commit to johnoliver/semantic-kernel that referenced this pull request Jun 5, 2024
@stephentoub @shawncal
Downgrade several 7.0.* packages to 6.0.* (microsoft#1799)
cad2c35 
This pains me, as it's a technical step backwards. But there are two
problems today:
1) The .NET support policy today unfortunately applies not only to the
.NET SDK and runtime but also to nuget packages released in the same
wave, which means that if someone is relying on eg System.Text.Json and
is running on .NET 6, having SK depend on System.Text.Json with a
minimum version of 7.0 forces the consumer to also use the 7.0 version
of System.Text.Json from the nuget, which then means that one assembly
falls under the STS support policy instead of LTS support policy, which
means that one assembly will have support end for it a few months
earlier than it otherwise would. I've raised this issue for further
evaluation on the .NET side of things, but in the meantime, this lowers
the version number to remove the perceived problem and possible barrier
to adoption.
2) Azure Functions has two deployment models: in-process and isolated.
Ideally functions use isolated, which gives them the freedom to
reference whatever they need. The in-process model is exactly what it
sounds like: the function runs in the same process as the host, and the
host pins several dependencies at 6.0 versions. That means that if a
function references a 7.0 version, it'll fail to load in the in-process
model. While we'd like for folks to be using the isolated model, we
can't force it, and we don't want a need for the in-process model to
block SK usage.

This commit downgrades back to the 6.0 versions, at least where
possible. Some of the connectors reference libraries (e.g. NRedisStack,
pgvector, etc.) that themselves have a 7.0 dependency.

Closes microsoft#1793

---------

Co-authored-by: Shawn Callegari <36091529+shawncal@users.noreply.github.com>
johnoliver pushed a commit to johnoliver/semantic-kernel that referenced this pull request Jun 5, 2024
@stephentoub @shawncal
Downgrade several 7.0.* packages to 6.0.* (microsoft#1799)
2a81aa9 
This pains me, as it's a technical step backwards. But there are two
problems today:
1) The .NET support policy today unfortunately applies not only to the
.NET SDK and runtime but also to nuget packages released in the same
wave, which means that if someone is relying on eg System.Text.Json and
is running on .NET 6, having SK depend on System.Text.Json with a
minimum version of 7.0 forces the consumer to also use the 7.0 version
of System.Text.Json from the nuget, which then means that one assembly
falls under the STS support policy instead of LTS support policy, which
means that one assembly will have support end for it a few months
earlier than it otherwise would. I've raised this issue for further
evaluation on the .NET side of things, but in the meantime, this lowers
the version number to remove the perceived problem and possible barrier
to adoption.
2) Azure Functions has two deployment models: in-process and isolated.
Ideally functions use isolated, which gives them the freedom to
reference whatever they need. The in-process model is exactly what it
sounds like: the function runs in the same process as the host, and the
host pins several dependencies at 6.0 versions. That means that if a
function references a 7.0 version, it'll fail to load in the in-process
model. While we'd like for folks to be using the isolated model, we
can't force it, and we don't want a need for the in-process model to
block SK usage.

This commit downgrades back to the 6.0 versions, at least where
possible. Some of the connectors reference libraries (e.g. NRedisStack,
pgvector, etc.) that themselves have a 7.0 dependency.

Closes microsoft#1793

---------

Co-authored-by: Shawn Callegari <36091529+shawncal@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@ericstj ericstj ericstj left review comments

@shawncal shawncal shawncal approved these changes

Assignees
No one assigned
Labels
kernel Issues or pull requests impacting the core kernel memory connector .NET Issue or Pull requests regarding .NET code
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Smeantic-Kernal NuGets fail in Function Apps
4 participants
@stephentoub @shawncal @Woland2k @ericstj