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

Experimental OperationStatus #1229

Closed
wants to merge 17 commits into from

Conversation

Wraith2
Copy link
Contributor

@Wraith2 Wraith2 commented Aug 25, 2021

I need to test these changes on CI to see if anything breaks that i can't test locally. Please ignore unless you're incredibly bored.

@Wraith2 Wraith2 marked this pull request as ready for review August 25, 2021 19:44
@Wraith2 Wraith2 closed this Aug 25, 2021
@Wraith2 Wraith2 reopened this Aug 25, 2021
@johnnypham
Copy link
Contributor

You can bring in the changes from #1251 to rerun CI.

@Wraith2
Copy link
Contributor Author

Wraith2 commented Sep 8, 2021

Thanks @johnnypham, there are still some errors. Cancel doesn't seem to work very well which may be a problem with this set of changes but that can be worked through. There are some total build failures though: C:\Users\dotnet\.nuget\packages\microsoft.sourcelink.common\1.0.0\build\InitializeSourceControlInformation.targets(3,81): Error MSB4022: The result "" of evaluating the value "$(_MicrosoftSourceLinkCommonAssemblyFile)" of the "AssemblyFile" attribute in element <UsingTask> is not valid. which I saw locally and definitely aren't something I've changed.
Overall I think that all that this needs is some attention on the cancel tests, thorough review and possible tweaking. Once that's done we'll have fixed the async string issue.

@Wraith2 Wraith2 closed this Sep 8, 2021
@Wraith2 Wraith2 reopened this Sep 9, 2021
@BradBarnich
Copy link

I tried this build out with the context switch set, and I get an error reading an xml column. With the switch off, it works.

System.Xml.XmlException
Unexpected end tag. Line 1, position 366.
   at System.Xml.XmlTextReaderImpl.Throw(Exception e)
   at System.Xml.XmlTextReaderImpl.Throw(String res, String arg)
   at System.Xml.XmlTextReaderImpl.ParseDocumentContent()
   at System.Xml.XmlTextReaderImpl.Read()
   at System.Xml.XmlWriter.WriteNode(XmlReader reader, Boolean defattr)
   at System.Data.SqlTypes.SqlXml.get_Value()
   at Microsoft.Data.SqlClient.SqlCachedBuffer.ToString()
   at Microsoft.Data.SqlClient.SqlBuffer.get_Value()
   at Microsoft.Data.SqlClient.SqlDataReader.GetValueFromSqlBufferInternal(SqlBuffer data, _SqlMetaData metaData)
   at Microsoft.Data.SqlClient.SqlDataReader.GetValueInternal(Int32 i)
   at Microsoft.Data.SqlClient.SqlDataReader.GetValue(Int32 i)

@Wraith2
Copy link
Contributor Author

Wraith2 commented Sep 26, 2021

Interesting, can you provide me with a minimal reproduction to debug against?

@BradBarnich
Copy link

Here you go

sql-xml-failure.zip

@Wraith2
Copy link
Contributor Author

Wraith2 commented Sep 27, 2021

Ok. That failure is sort of expected because I haven't tested that route through the library. Handling the xml type is totally different to handing strings or binary chunks and it just hasn't been reviewed. That doesn't mean it can't be made to work but at the moment I just don't have the energy to put into it, it's taken me a year to get this far and I need a break from tracking down async in this library.

@Wraith2 Wraith2 closed this Oct 19, 2021
@Wraith2 Wraith2 deleted the experimental-operationstatus branch October 19, 2021 09:15
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants