add .Net6 support across codebase by martijn00 · Pull Request #732 · minio/minio-dotnet

martijn00 · 2023-01-13T09:13:41Z

This removes dependencies on old libraries: fixes #725

martijn00 · 2023-01-24T15:47:03Z

@ebozduman @harshavardhana would you mind giving this a review? I can make changes if required.

ebozduman · 2023-01-26T05:25:30Z

@martijn00
Sure. Will do.

ebozduman

@martijn00 ,

There are lint errors.
Please run dotnet regitlint and then push those lint changes

martijn00 · 2023-02-07T19:54:06Z

@ebozduman Done

ebozduman

@martijn00

Please check the build test run failure logs.
There are quite a lot of warnings and errors, which need to be addressed, for Minio.Tests.

martijn00 · 2023-02-16T14:45:39Z

@ebozduman Did you look at the error? It seems the connection get refused. Are the credentials still correct?

These test fail on master branch too!

This is currently used:

        _minioClient = new MinioClient()
            .WithEndpoint("play.min.io")
            .WithCredentials("Q3AM3UQ867SPQQA43P2F",
                "zuf+tfteSlswRu7BJ86wekitnifILbZam1KYY3TG")

ebozduman

@martijn00 ,

With your recent changes, it is back to lint errors in 38 files.
No need to say it, but we need to run dotnet regitlint before pushing any change.

martijn00 · 2023-02-17T13:06:45Z

@ebozduman I've updated the PR again. My question still remains: It seems the connection get refused. Are the credentials still correct?

I don't think the Test will ever work if that is not fixed on your side.

martijn00 · 2023-02-17T19:08:18Z

@ebozduman I've switch the failing test over to use ssl, and now they run. Can you have a look again?

martijn00 · 2023-02-20T14:15:19Z

@ebozduman Locally I have everything running. Can you try run the tests again with the latest change? See if that helps

I still see lots of room for improvement but i don't want to make this PR bigger than it already is. Let's work together to get this merged, and then continue from there.

ebozduman · 2023-02-22T06:00:39Z

@martijn00
Sorry for forgetting to answer your question;

My question still remains: It seems the connection get refused. Are the credentials still correct?

Yes, there was a change in play's setup. We were supposed to hit https, and you are right; this issue does not belong to your change.
So, I've created a separate PR, #749, and asked expedited review from the reviewer.

ebozduman · 2023-02-22T21:26:47Z

@martijn00,

Please rebase. PR#749 is merged.

martijn00 · 2023-02-28T17:03:13Z

@ebozduman can you try again now?

Any other comments or remarks, or things you want explanation?

ebozduman · 2023-03-01T04:29:15Z

Hey @martijn00 ,

I've a couple of comments to share;

1. I see some warning messages about the removed packages, like System or System.Collections.Generic. But the workflow build test for 7.0 passes. There maybe something wrong in my env. I'll check it out.
Yes. It was the OmniSharp process and it was giving me wrong warning messages. It needed to be restarted.

The PASS/FAIL output messages are now prettified with indentations, and etc.
I think the default should have been the regular one-liner outputs as before for regression purposes. If a user wants to see the prettified output, s/he can turn it on.
Could you revert it back, so that we can see one-liner test result output for each test ?
I am also going to add .Net6.0 as part of the build process and testing. It is a required step.

martijn00 · 2023-03-01T18:22:26Z

@ebozduman Ok, done comment 1. When can you get to comment 2?

ebozduman · 2023-03-01T22:01:42Z

@martijn00 Done. Build tests are running right now

ebozduman · 2023-03-02T00:10:35Z

@martijn00

I forgot to ask. Why is Docs/API.md removed?

martijn00 · 2023-03-02T11:02:50Z

@ebozduman not sure why. I added it back now.

ebozduman

LGTM

ebozduman · 2023-03-14T01:20:45Z

Hi @martijn00

I wanted to let you know that we missed to check 2 areas for this PR and there we now see regression;

The release script that creates the MinIO nupackage is broken as the script expects to find a Version tag, the version of the next release information, in the Minio.csprpoj file.
MinIO supports not only .Net6.0 and .Net7.0 , but also .Net4.x.
So, we were supposed to run the functional tests against .Net4.x release(s).
I see quite a lot of build errors when I run the functional tests against .Net4.7.2 or .Net4.8, mostly because some new features introduced in .Net6.0 are not available for .Net4.x and the versions of some packages are not comopatible with .Net4.x.

Please let us know what your opinion is about these 2 regressions and possible fixes for them.
Thanks

martijn00 · 2023-03-14T10:03:43Z

@ebozduman Where can i find this script? The version is now here: https://github.com/minio/minio-dotnet/blob/master/Directory.Build.props#L19 It should be possible to point it there?

Looking at the LTS versions of .NET it would be best to drop support for .NET4.x: https://dotnet.microsoft.com/en-us/platform/support/policy/dotnet-core

Are there any compelling reasons to still support really old frameworks like that?

Also can you look at: #754

ebozduman · 2023-03-14T21:45:37Z

The version is now here: https://github.com/minio/minio-dotnet/blob/master/Directory.Build.props#L19 It should be possible to point it there?

Yes, the script'll point to Directory.Build.props.
We'll take care of the script change The release script is in a private repo.

Abour .Net4.x:
We are also discussing the drop of .Net4.x support.

I'll get to #754 as soon as I can.

ebozduman · 2023-03-15T22:20:08Z

@martijn00

re: .Net4.x support
Even if we decide to drop the support for .Net4.x, we still need to make the changes as it will take some time to drop it after announcement.

martijn00 · 2023-03-15T22:35:31Z

What sort of changes? Also, users can still use older nuget packages for net4.

ebozduman · 2023-03-16T16:41:40Z

What sort of changes?

To make it work with the latest MinIO nuget package.

Also, users can still use older nuget packages for net4.

Yes they can, but it is a workaround, which means we don't support .Net4.x

martijn00 · 2023-03-16T19:21:46Z

But what sort of code changes are that? I can't imagine anything... Can I help with it if you could elaborate?

ebozduman · 2023-03-22T00:20:38Z

Hi @martijn00

They are build errors, like for unknown attribute names, etc.
I run MinIO functional tests against .Net4.8 and .Net4.7.2.
Some of those build errors are listed below:

$ dotnet run --project Minio.Functional.Tests/Minio.Functional.Tests.csproj 
/home/ersan/work/src/github.com/minio/minio-dotnet/Minio/Credentials/ECSCredentials.cs(18,19): error CS0234: The type or namespace name 'Json' does not exist in the namespace 'System.Text' (are you missing an assembly reference?) [/home/ersan/work/src/github.com/minio/minio-dotnet/Minio/Minio.csproj::TargetFramework=net4.8]
/home/ersan/work/src/github.com/minio/minio-dotnet/Minio/Credentials/IAMAWSProvider.cs(19,19): error CS0234: The type or namespace name 'Json' does not exist in the namespace 'System.Text' (are you missing an assembly reference?) [/home/ersan/work/src/github.com/minio/minio-dotnet/Minio/Minio.csproj::TargetFramework=net4.8]
/home/ersan/work/src/github.com/minio/minio-dotnet/Minio/Credentials/JsonWebToken.cs(18,19): error CS0234: The type or namespace name 'Json' does not exist in the namespace 'System.Text' (are you missing an assembly reference?) [/home/ersan/work/src/github.com/minio/minio-dotnet/Minio/Minio.csproj::TargetFramework=net4.8]
/home/ersan/work/src/github.com/minio/minio-dotnet/Minio/DataModel/MinioNotification.cs(18,19): error CS0234: The type or namespace name 'Json' does not exist in the namespace 'System.Text' (are you missing an assembly reference?) [/home/ersan/work/src/github.com/minio/minio-dotnet/Minio/Minio.csproj::TargetFramework=net4.8]
/home/ersan/work/src/github.com/minio/minio-dotnet/Minio/Helper/utils.cs(23,19): error CS0234: The type or namespace name 'Json' does not exist in the namespace 'System.Text' (are you missing an assembly reference?) [/home/ersan/work/src/github.com/minio/minio-dotnet/Minio/Minio.csproj::TargetFramework=net4.8]
/home/ersan/work/src/github.com/minio/minio-dotnet/Minio/HttpRequestMessageBuilder.cs(18,18): error CS0234: The type or namespace name 'Http' does not exist in the namespace 'System.Net' (are you missing an assembly reference?) [/home/ersan/work/src/github.com/minio/minio-dotnet/Minio/Minio.csproj::TargetFramework=net4.8]
/home/ersan/work/src/github.com/minio/minio-dotnet/Minio/V4Authenticator.cs(19,19): error CS0234: The type or namespace name 'Json' does not exist in the namespace 'System.Text' (are you missing an assembly reference?) 
:
:
:
The build failed. Fix the build errors and run again.

You just change the target framework to .net4.7.2 and .Net4.8 in Minio.Functional.Tests.csproj file and run the functional tests. You'll see the error messages.

martijn00 · 2023-03-23T11:53:30Z

@ebozduman ok, so you want to support .net4.8? I thought you are discussing dropping the support?

There is no point in maintaining net48 support anymore. You are blocking real support for .net5, 6, 7 and 8 with this.
As said before, users still on net4.8 or lower can just use the older nuget packages. If problems come up you can still release a 4.x version with support for older frameworks, while the newer Minio 5.x versions only work on newer .net.

These things are not regressions, it's just the future.

ebozduman · 2023-03-24T22:42:44Z

I agree with all the points you've made, but we have to follow the process.
My understanding is as long as Microsoft supports, we need to continue supporting those releases and Microsoft supports 4.7 and 4.8.
We are currently discussing it.

martijn00 · 2023-03-24T22:45:47Z

If it helps, I could do a call with you and management to talk about it. Let me know if you want that.

Co-authored-by: ebozduman

ebozduman suggested changes Feb 7, 2023

View reviewed changes

ebozduman suggested changes Feb 14, 2023

View reviewed changes

martijn00 requested a review from ebozduman February 16, 2023 16:22

ebozduman suggested changes Feb 17, 2023

View reviewed changes

martijn00 requested a review from ebozduman February 17, 2023 13:07

martijn00 mentioned this pull request Feb 20, 2023

net462 minio putobject objectkey Include parentheses "()" error #708

Closed

martijn00 added 15 commits February 22, 2023 22:33

Convert to json net

6dfe822

Fix last stuff

3c2fb1e

Update packages

fe3433f

Update minio-dotnet-windows.yml

548a66f

Update minio-dotnet-linux.yml

0eb6840

Run dotnet regitlint

6bb3193

Update package

1937219

Fix usings

cac1d5c

Clean proj

fd54815

Fix messages

200bf1b

Fix warnings

b2b7094

Update tools

b17f34f

Formatting

5550214

Warning fixes

e3000a1

Formatting

de4f9bb

Disable pretty json

df5c859

Adds 6.0.x to build version

cefeb7c

ebozduman approved these changes Mar 1, 2023

View reviewed changes

ebozduman requested a review from harshavardhana March 2, 2023 00:00

Add docs back

fc78579

clean up print out

0006643

ebozduman approved these changes Mar 4, 2023

View reviewed changes

harshavardhana approved these changes Mar 4, 2023

View reviewed changes

harshavardhana changed the title ~~Update to .Net6~~ add .Net6 support across codebase Mar 4, 2023

harshavardhana merged commit b332b78 into minio:master Mar 4, 2023

martijn00 mentioned this pull request Mar 23, 2023

Netstandard compat #756

Closed

martijn00 mentioned this pull request Apr 13, 2023

Version 4.0.4 - Strong name signature could not be verified #661

Closed

Urantij pushed a commit to Urantij/minio-dotnet that referenced this pull request Apr 19, 2023

add .Net6 support across codebase (minio#732)

7072610

Co-authored-by: ebozduman

Conversation

martijn00 commented Jan 13, 2023

Uh oh!

martijn00 commented Jan 24, 2023

Uh oh!

ebozduman commented Jan 26, 2023

Uh oh!

ebozduman left a comment

Choose a reason for hiding this comment

Uh oh!

martijn00 commented Feb 7, 2023

Uh oh!

ebozduman left a comment

Choose a reason for hiding this comment

Uh oh!

martijn00 commented Feb 16, 2023

Uh oh!

ebozduman left a comment

Choose a reason for hiding this comment

Uh oh!

martijn00 commented Feb 17, 2023

Uh oh!

martijn00 commented Feb 17, 2023

Uh oh!

martijn00 commented Feb 20, 2023

Uh oh!

ebozduman commented Feb 22, 2023

Uh oh!

ebozduman commented Feb 22, 2023

Uh oh!

martijn00 commented Feb 28, 2023

Uh oh!

ebozduman commented Mar 1, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

martijn00 commented Mar 1, 2023

Uh oh!

ebozduman commented Mar 1, 2023

Uh oh!

ebozduman commented Mar 2, 2023

Uh oh!

martijn00 commented Mar 2, 2023

Uh oh!

ebozduman left a comment

Choose a reason for hiding this comment

Uh oh!

ebozduman commented Mar 14, 2023

Uh oh!

martijn00 commented Mar 14, 2023

Uh oh!

ebozduman commented Mar 14, 2023

Uh oh!

ebozduman commented Mar 15, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

martijn00 commented Mar 15, 2023

Uh oh!

ebozduman commented Mar 16, 2023

Uh oh!

martijn00 commented Mar 16, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

ebozduman commented Mar 22, 2023

Uh oh!

martijn00 commented Mar 23, 2023

Uh oh!

ebozduman commented Mar 24, 2023

Uh oh!

martijn00 commented Mar 24, 2023

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

ebozduman commented Mar 1, 2023 •

edited

Loading

ebozduman commented Mar 15, 2023 •

edited

Loading

martijn00 commented Mar 16, 2023 •

edited

Loading