[CLIENT-2322] Add support for Windows #437

juliannguyen4 · 2023-05-02T21:55:57Z

NOTE: wheels are built on Windows 2022, which is not the same as Windows 10.

Build wheels all passes and uploaded to JFrog: https://github.com/aerospike/aerospike-client-python/actions/runs/7674493826/
Valgrind, no memory errors or leaks related to PR: https://github.com/aerospike/aerospike-client-python/actions/runs/7644298063/job/20828308036

codecov-commenter · 2023-05-02T22:20:49Z

Codecov Report

Attention: 16 lines in your changes are missing coverage. Please review.

Comparison is base (31add98) 81.25% compared to head (32694b7) 81.39%.

❗ Current head 32694b7 differs from pull request most recent head fccfe8e. Consider uploading reports for the commit fccfe8e to get more accurate results

Files	Patch %	Lines
src/main/client/cdt_list_operate.c	0.00%	8 Missing ⚠️
src/main/client/batch_operate.c	50.00%	2 Missing ⚠️
src/main/client/batch_remove.c	0.00%	2 Missing ⚠️
src/main/convert_expressions.c	99.10%	2 Missing ⚠️
src/main/client/batch_apply.c	80.00%	1 Missing ⚠️
src/main/conversions.c	80.00%	1 Missing ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##            stage     #437      +/-   ##
==========================================
+ Coverage   81.25%   81.39%   +0.13%     
==========================================
  Files          99       98       -1     
  Lines       14897    14894       -3     
==========================================
+ Hits        12105    12123      +18     
+ Misses       2792     2771      -21

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

This reverts commit b79c4a9.

This reverts commit f6796a1.

dwelch-spike

Mostly style suggestions. Have a look and let me know what you think.

dwelch-spike · 2024-02-07T19:25:03Z

setup.py

+    AEROSPIKE_C_TARGET = AEROSPIKE_C_HOME
+    libraries.clear()
+    extra_compile_args.append("-DAS_SHARED_IMPORT")
+    include_dirs.append(AEROSPIKE_C_TARGET + "/vs/packages/aerospike-client-c-dependencies.1.0.2/build/native/include")


Will this path ever change? For example might the 1.0.2 change with an update? If so maybe this path needs to be built dynamically.

dwelch-spike · 2024-02-07T19:26:23Z

setup.py

+    ]
+else:
+    include_dirs.append(AEROSPIKE_C_TARGET + '/src/include')
+    library_dirs.append(AEROSPIKE_C_TARGET + "/vs/packages/aerospike-client-c-dependencies.1.0.2/build/native/lib/x64/Release")


Same path question here.

dwelch-spike · 2024-02-07T19:33:01Z

src/include/conversions.h

+as_status strArray_to_py_list(as_error *err, int num_elements,
+                              char str_array_ptr[][AS_ROLE_SIZE],


Why was this change made? Is this function only ever used for roles? If so I think it should be renamed.

See this comment

src/main/client/type.c

dwelch-spike · 2024-02-07T19:39:36Z

src/main/conversions.c

+as_status strArray_to_py_list(as_error *err, int num_elements,
+                              char str_array_ptr[][AS_ROLE_SIZE],


If this assumes it is working with roles I think it should be renamed so that it doesn't look general purpose.

Got it. If I remember correctly, I made this change because the Visual Studio compiler was complaining about taking in a variable size array.

dwelch-spike · 2024-02-07T19:47:45Z

src/main/convert_expressions.c

-        [NOT] = EXP_SZ(as_exp_not({})),
+            as_exp_nil(),
+            as_exp_nil())), // ^ TODO implement a less wastefull solution.
+        [NE] = EXP_SZ(as_exp_cmp_ne(as_exp_nil(), as_exp_nil())),


All the as_exp_nil() calls reduce readability in an already dense file. I think a macro that wraps as_exp_nil would help. Maybe #define NIL as_exp_nil().

juliannguyen4 · 2024-02-07T22:24:33Z

New changes pass testing on Windows: https://github.com/aerospike/aerospike-client-python/actions/runs/7821045905/job/21339450593

dwelch-spike

LGTM

juliannguyen4 force-pushed the CLIENT-2322-windows-support branch 2 times, most recently from 016cf22 to 167c6de Compare May 9, 2023 00:53

juliannguyen4 force-pushed the CLIENT-2322-windows-support branch from 0cceb95 to ab46d07 Compare May 18, 2023 22:05

juliannguyen4 force-pushed the CLIENT-2322-windows-support branch from d4f55fd to 4fcf564 Compare May 26, 2023 16:38

juliannguyen4 force-pushed the CLIENT-2322-windows-support branch 2 times, most recently from 0b9846a to c25a487 Compare November 15, 2023 23:44

juliannguyen4 mentioned this pull request Jan 19, 2024

Windows support / compability #436

Closed

juliannguyen4 added 22 commits January 22, 2024 09:44

Add windows cicd job

e92ab63

use default 3.9

8b8c74e

try installing setuptools

d17a89d

try removing include dirs

884dbbc

Add windows classifier

2b86bfc

use setup.py directly

383e21c

try older windows

39c7b89

Don't throw error when windows 10 runs setup.py

1a3ce92

use build frontend again

37d7a74

Revert "try older windows"

c1d09b2

This reverts commit b79c4a9.

Specify C client artifacts for windows

57e53d4

Don't run rest of setup script for windows

d434b5c

add back some code for windows

716cc6b

Remove unsupported flag in windows

853b50e

Remove unsupported flag in windows again

60dee5e

Build visual studio target

64ef9e8

add msbuild to path

21ef8b4

release is config name, not target

bb87114

Install C client dependencies using nuget cli

718a588

Attempt to compile on windows

1739cf6

Remove info_node(), use windows native atomic function

4b11eea

finally works

b953c06

juliannguyen4 added 6 commits January 25, 2024 16:34

fix bug on windows

45c2982

Revert "debug"

0ac392e

This reverts commit f6796a1.

fix bugs

f16a39f

fix

f944973

fix for linux and windows

7acd25a

use macro instead of outright replacing the function

b40eb49

juliannguyen4 force-pushed the CLIENT-2322-windows-support branch from cc245df to b40eb49 Compare January 26, 2024 16:50

juliannguyen4 added 10 commits January 26, 2024 09:33

set same type

33b0f98

precommit format

fd51f3b

fix test bug

d56f4ee

rm dead code

353151c

check for -10 and 9

6754049

fix test

9065c6e

cleanup setup script

73ee119

not sure why i commented these out

8508fc0

remove unused enum

32694b7

fix linker error

fccfe8e

juliannguyen4 requested a review from dwelch-spike January 26, 2024 23:19

Bump version to 15.0.0-rc.2

6e6102f

juliannguyen4 requested a review from justinlee-aerospike January 27, 2024 00:27

update readme

b5f7c90

dwelch-spike requested changes Feb 7, 2024

View reviewed changes

juliannguyen4 added 3 commits February 7, 2024 12:41

Rename function that converts list of roles in C to Python

d1340d1

Improve readability of convert_expressions.c

c34305b

Dynamically fetch version of C client dependencies for Windows

e66f38e

juliannguyen4 requested a review from dwelch-spike February 7, 2024 22:24

dwelch-spike approved these changes Feb 7, 2024

View reviewed changes

juliannguyen4 merged commit 8aa7e68 into stage Feb 7, 2024
56 checks passed

juliannguyen4 deleted the CLIENT-2322-windows-support branch February 7, 2024 23:06

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[CLIENT-2322] Add support for Windows #437

[CLIENT-2322] Add support for Windows #437

juliannguyen4 commented May 2, 2023 •

edited

Loading

codecov-commenter commented May 2, 2023 •

edited

Loading

dwelch-spike left a comment

dwelch-spike Feb 7, 2024

juliannguyen4 Feb 7, 2024

dwelch-spike Feb 7, 2024

juliannguyen4 Feb 7, 2024

dwelch-spike Feb 7, 2024

juliannguyen4 Feb 7, 2024

dwelch-spike Feb 7, 2024

juliannguyen4 Feb 7, 2024

dwelch-spike Feb 7, 2024

juliannguyen4 Feb 7, 2024

juliannguyen4 commented Feb 7, 2024

dwelch-spike left a comment

		as_status strArray_to_py_list(as_error *err, int num_elements,
		char str_array_ptr[][AS_ROLE_SIZE],

[CLIENT-2322] Add support for Windows #437

[CLIENT-2322] Add support for Windows #437

Conversation

juliannguyen4 commented May 2, 2023 • edited Loading

codecov-commenter commented May 2, 2023 • edited Loading

Codecov Report

dwelch-spike left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

juliannguyen4 commented Feb 7, 2024

dwelch-spike left a comment

Choose a reason for hiding this comment

juliannguyen4 commented May 2, 2023 •

edited

Loading

codecov-commenter commented May 2, 2023 •

edited

Loading