CSharp integration #299

OrangeSmokingJacket · 2023-12-30T11:50:02Z

Name change leftovers
Start of a C# integration, which will be expanded later.
Now able to:
Create a database and a collection
Insert, update, delete and select documents via ExecuteSQL()
Dockerfiles are now 2 stage builds, which lowers final image size and simplifies dotnet usage

.github/workflows/csharp.yaml

docker/Dockerfile-ubuntu-20

SergeiNA · 2023-12-31T00:23:07Z

docker/Dockerfile-ubuntu-20

+        python3-dev curl gnupg apt-transport-https \
+        zlib1g \
+        wget && \
+    wget https://packages.microsoft.com/config/ubuntu/20.04/packages-microsoft-prod.deb -O packages-microsoft-prod.deb && \


May be we can wrap all cs dependeces intro one script

https://hub.docker.com/_/microsoft-dotnet-sdk

https://hub.docker.com/_/microsoft-dotnet-sdk

But it is not possible to run docker commands inside a dockerfile. Or is it?

Just use it for FROM directive at the top and start with the container, full of M$ libs.

In this case it should be a separate dockerfile isn't it?
cc @CLTanuki @OrangeSmokingJacket

SergeiNA · 2023-12-31T00:27:19Z

docker/Dockerfile-ubuntu-20

+RUN cd integration/python/ && pytest
+
+RUN cd .. && \


@OrangeSmokingJacket @kotbegemot IMHAO I'd like to see separate docker file for dotnet. Or it should be in each docker file.
@kotbegemot I think we need to rethink our approach to docker files infrastructure.

So what thoughts on it?

SergeiNA · 2023-12-31T00:29:10Z

components/cursor/cursor.hpp

@@ -21,16 +21,16 @@ namespace components::cursor {

    enum class operation_status_t { success = 1, failure = 0 };

-    enum class error_code_t {
+    enum class error_code_t : int32_t {


Do we really need 32 bits here?

It is for C# conversion, but i think it will work with less bytes

For some weird reason, 32 bits are not necessary for C++, but other types behaves strange in C#...
is_error() function has this code: { return error_.type != error_code_t::none; }, and it works fine with all singed and unsigned types, but C#, which just calls that function returns true even if error_.type is "none" for every type, exept int32_t. And i couldn't figure out why...

If it's require by c# fine but plz leave a comment like "We have to use int32_t because of c# comparability type restrictions"

@OrangeSmokingJacket

components/cursor/cursor.hpp

components/cursor/cursor.cpp

components/document/document_view.hpp

services/collection/collection.cpp

integration/cpp/test/test_collection_sql.cpp

integration/cpp/test/test_collection_ql.cpp

integration/c/CMakeLists.txt

integration/c/otterbrix.h

CLTanuki · 2024-01-04T18:53:39Z

integration/csharp/src/Duckstax.Otterbrix/Duckstax.Otterbrix.csproj

@@ -4,6 +4,12 @@
        <TargetFramework>net7.0</TargetFramework>
        <ImplicitUsings>enable</ImplicitUsings>
        <Nullable>enable</Nullable>
+        <AllowUnsafeBlocks>true</AllowUnsafeBlocks>


Do not think, it is necessary, because project successfully builds without it.

CLTanuki · 2024-01-04T18:59:55Z

integration/csharp/src/Duckstax.Otterbrix/OtterbrixWrapper.cs

+                   CallingConvention = CallingConvention.Cdecl)]
+        private static extern IntPtr
+        OtterbrixCreate(TransferConfig config, StringPasser database, StringPasser collection);
+        [DllImport(libotterbrix,


There should be empty lines between functions. Take on of public editorconfigs like this and add <TreatWarningsAsErrors>true</TreatWarningsAsErrors> to csproj to avoid having to remember these rules.

CLTanuki · 2024-01-04T19:02:44Z

integration/csharp/tests/Duckstax.Otterbrix.Tests/WrapperTest.cs

+            CursorWrapper cursor = otterbrix.Execute(query);
+            Assert.IsTrue(cursor != null);
+            Assert.IsTrue(cursor.IsSuccess());
+            Assert.IsFalse(cursor.IsError());


I think it cannot error if previous assertion passes - it's not a Shroedinger `s cat. Same for other assertions. It can save some CI time.

The way i build it, it can, there is a case, when IsSuccess() will be false, but IsError() will also be false.
If we select something but nothing was selected, operation won't be successful, because nothing was selected, but also there won't be any errors, because everything worked fine.
It done that way to simplify some checks when there is need to see if something was selected or not

CLTanuki · 2024-01-04T19:03:52Z

integration/csharp/tests/Duckstax.Otterbrix.Tests/WrapperTest.cs

+            {
+                DocumentWrapper doc = cursor.Next();
+                Assert.IsTrue(doc != null);
+                Assert.IsTrue(doc.GetLong("count") == 99);
+                Assert.IsTrue(doc.GetString("name") == "Name 99");
+            }
+            {
+                DocumentWrapper doc = cursor.Next();
+                Assert.IsTrue(doc != null);
+                Assert.IsTrue(doc.GetLong("count") == 98);
+                Assert.IsTrue(doc.GetString("name") == "Name 98");
+            }
+            {
+                DocumentWrapper doc = cursor.Next();
+                Assert.IsTrue(doc != null);
+                Assert.IsTrue(doc.GetLong("count") == 97);
+                Assert.IsTrue(doc.GetString("name") == "Name 97");
+            }
+            {
+                DocumentWrapper doc = cursor.Next();
+                Assert.IsTrue(doc != null);
+                Assert.IsTrue(doc.GetLong("count") == 96);
+                Assert.IsTrue(doc.GetString("name") == "Name 96");
+            }
+            {
+                DocumentWrapper doc = cursor.Next();
+                Assert.IsTrue(doc != null);
+                Assert.IsTrue(doc.GetLong("count") == 95);
+                Assert.IsTrue(doc.GetString("name") == "Name 95");
+            }


How about a for loop? Possibly You can end with TestCase parameters - it's easier to read and maintain.

CLTanuki · 2024-01-04T19:04:13Z

integration/csharp/tests/Duckstax.Otterbrix.Tests/WrapperTest.cs

+            {
+                DocumentWrapper doc = cursor.Next();
+                Assert.IsTrue(doc != null);
+                Assert.IsTrue(doc.GetLong("count") == 0);
+                Assert.IsTrue(doc.GetString("name") == "Name 0");
+            }
+            {
+                DocumentWrapper doc = cursor.Next();
+                Assert.IsTrue(doc != null);
+                Assert.IsTrue(doc.GetLong("count") == 1);
+                Assert.IsTrue(doc.GetString("name") == "Name 1");
+            }
+            {
+                DocumentWrapper doc = cursor.Next();
+                Assert.IsTrue(doc != null);
+                Assert.IsTrue(doc.GetLong("count") == 2);
+                Assert.IsTrue(doc.GetString("name") == "Name 2");
+            }
+            {
+                DocumentWrapper doc = cursor.Next();
+                Assert.IsTrue(doc != null);
+                Assert.IsTrue(doc.GetLong("count") == 3);
+                Assert.IsTrue(doc.GetString("name") == "Name 3");
+            }
+            {
+                DocumentWrapper doc = cursor.Next();
+                Assert.IsTrue(doc != null);
+                Assert.IsTrue(doc.GetLong("count") == 4);
+                Assert.IsTrue(doc.GetString("name") == "Name 4");
+            }


Same about loop.

SergeiNA · 2024-01-07T03:33:58Z

docker/Dockerfile-ubuntu-20

@@ -1,4 +1,4 @@
-FROM ubuntu:20.04
+FROM ubuntu:20.04 as builder


Looks like you use here Multi-stage builds, plz add info about in into PR description.
Also @kotbegemot PTAL, as more I see cases like this more I want to use separate dockerfile for C# buuut... We need to discuss it.

docker/Dockerfile-ubuntu-20

SergeiNA

From my side we need to discus only dockerfile structure. No more significant notes. So fix all c# issue and merge it.

SergeiNA · 2024-01-07T03:39:25Z

Also I saw
dotnet package / Run (3.1.x, ubuntu-20.04, 3.8) (pull_request)
dotnet package / Run (6.0.x, ubuntu-20.04, 3.8) (pull_request)

Is it ok to have both of them? And they both failed.
cc @CLTanuki @OrangeSmokingJacket @kotbegemot

SergeiNA · 2024-01-12T00:42:29Z

@kotbegemot do we need C# tests for manylinux?

@OrangeSmokingJacket I still see falling
dotnet package / Run (3.1.x, ubuntu-20.04, 3.8) (pull_request)
and why you turn off
dotnet package / Run (6.0.x, ubuntu-20.04, 3.8) (pull_request) ?
Plz add comment.

OrangeSmokingJacket · 2024-01-12T14:54:10Z

@kotbegemot do we need C# tests for manylinux?

@OrangeSmokingJacket I still see falling dotnet package / Run (3.1.x, ubuntu-20.04, 3.8) (pull_request) and why you turn off dotnet package / Run (6.0.x, ubuntu-20.04, 3.8) (pull_request) ? Plz add comment.

There are 2 test cases in each (the first one is from WrapperTest.cs, which works fine, the second is for EntityFramework, which is not implemented yet).
As far as I know, 3.1.x and 6.0.x are running at the same time and failed one cancels the other one

kotbegemot · 2024-01-12T16:42:44Z

@SergeiNA manylinux is only needed to build packages for pypi on the linux

OrangeSmokingJacket added 12 commits December 20, 2023 17:32

name chaned (duckstax#275) leftovers

e081e21

name chaned (duckstax#275) leftovers

9c82d9f

name chaned (duckstax#275) leftovers

f4cb8ad

csharp tests added

b30e4a8

CSharp integration

c2baf44

CSharp integration

883588b

CSharp integration

de63ca2

CSharp integration

814af8d

CSharp integration

ee90697

CSharp integration

572a682

CSharp integration

65440b6

c++ tests update

8d777c7