python3Packages.python-arango: init at 7.5.1 by jsoo1 · Pull Request #193783 · NixOS/nixpkgs

jsoo1 · 2022-09-30T22:46:05Z

Description of changes

Introduce the python ArangoDB driver library.

Things done

WolfangAukang · 2022-10-01T00:08:04Z

The library has tests on the main repository. It would be recommended to use fetchFromGitHub instead.

pkgs/development/python-modules/python-arango/default.nix

jsoo1 · 2022-10-25T20:02:36Z

I'm waiting to merge #194670 before this goes in. Plus I'd like to understand the test failures more before I am confident this library is ok.

jsoo1 · 2022-11-08T14:27:52Z

@WolfangAukang I was able to reenable a bunch of tests and update the commentary. Some of these test failures look like things that were missed upstream, some are due to the sandbox. I am much more confident this is ready. Plus #194670 was merged.

WolfangAukang · 2022-11-08T16:37:15Z

Result of nixpkgs-review pr 193783 run on x86_64-linux 1

2 packages failed to build:

python310Packages.python-arango
python39Packages.python-arango

error: builder for '/nix/store/7yw3hcnrpqzg76zcng2f8y1kgi1ib37z-python3.9-python-arango-7.5.1.drv' failed with exit code 3;
       last 10 log lines:
       > INTERNALERROR>   File "/build/source/arango/api.py", line 74, in _execute
       > INTERNALERROR>     return self._executor.execute(request, response_handler)
       > INTERNALERROR>   File "/build/source/arango/executor.py", line 64, in execute
       > INTERNALERROR>     resp = self._conn.send_request(request)
       > INTERNALERROR>   File "/build/source/arango/connection.py", line 446, in send_request
       > INTERNALERROR>     return self.process_request(host_index, request)
       > INTERNALERROR>   File "/build/source/arango/connection.py", line 154, in process_request
       > INTERNALERROR>     raise ConnectionAbortedError(
       > INTERNALERROR> ConnectionAbortedError: Can't connect to host(s) within limit (9)
       > /nix/store/6gqfxldwhp3sm2wr6r9piiyd0s8sjchh-pytest-check-hook/nix-support/setup-hook: line 53:   166 Illegal instruction     (core dumped) ICU_DATA=/nix/store/ya930rs3yx8cs7bqrxlc7pd8p7aics5n-arangodb-3.10.0/share/arangodb3 GLIBCXX_FORCE_NEW=1 TZ=UTC TZ_DATA=/nix/store/ya930rs3yx8cs7bqrxlc7pd8p7aics5n-arangodb-3.10.0/share/arangodb3/tzdata ARANGO_ROOT_PASSWORD=test /nix/store/ya930rs3yx8cs7bqrxlc7pd8p7aics5n-arangodb-3.10.0/bin/arangod --server.uid=$(id -u) --server.gid=$(id -g) --server.authentication=true --server.endpoint=http+tcp://127.0.0.1:8529 --server.descriptors-minimum=4096 --server.jwt-secret=secret --javascript.app-path=.nix-test/app --log.file=.nix-test/log --database.directory=.nix-test/data --foxx.api=false

jsoo1 · 2022-11-09T02:25:57Z

@WolfangAukang

166 Illegal instruction

This suggests to me this ran into the portability issues I mentioned here: arangodb/arangodb#17454

Can you do me a favor and send the output of

awk '
  /^(cpu family|vendor_id)[ \t]+:/ { print $0 }
  /^model[ \t]+:/ { printf("%s (%x)\n", $0, $3) }
' /proc/cpuinfo

please?

WolfangAukang · 2022-11-09T12:39:58Z

Can you do me a favor and send the output of

vendor_id	: GenuineIntel
cpu family	: 6
model		: 58 (3a)

Not sure if this is the expected response.

jsoo1 · 2022-11-09T17:17:12Z

Can you do me a favor and send the output of
vendor_id	: GenuineIntel
cpu family	: 6
model		: 58 (3a)
Not sure if this is the expected response.

Thank you! I wanted to check what model cpu you have. (arangodb is configured to build for haswell by default - which is model 3f). 3a (what you have) is ivy-bridge. I had some debate internally about what architecture to default to. It is just not possible to compile arangodb reproducibly without specifying the cpu architecture up front. I wonder if the default architecture should be changed.

Note: on haswell the other two packages that failed for you do succeed.

Edit: Since this is a client library, the architecture issue probably does not matter (the python driver would talk over the network to arangodb). It would be nice to disable the tests conditionally because of the lack of arangodb portability.

WolfangAukang · 2022-11-09T20:55:13Z

That makes sense. Like you mentioned, this is the client and the error was related to the database. I haven't had a case where the CPU model needs to be considered, so I can't provide more input unfortunately.

jsoo1 · 2022-11-11T18:42:51Z

That makes sense. Like you mentioned, this is the client and the error was related to the database. I haven't had a case where the CPU model needs to be considered, so I can't provide more input unfortunately.

Ok. In that case I'll disable tests but keep the test code around for the next time I need to upgrade.

nixos-discourse · 2022-11-16T21:09:28Z

This pull request has been mentioned on NixOS Discourse. There might be relevant details there:

https://discourse.nixos.org/t/prs-ready-for-review/3032/1436

SuperSandro2000

Your attribute ordering is a bit off. It should be after src like:

src
propagatedBuildInputs
checkInputs
preCheck
pytestFlagsArray
pythonImportsCheck
meta

SuperSandro2000 · 2022-11-17T00:54:02Z

pkgs/development/python-modules/python-arango/default.nix

Suggested change

#

# nonetheless, the client library should remain in nixpkgs - since

# the client library will talk to arangodb across the network and

# architecture issues will be irrelevant.

SuperSandro2000 · 2022-11-17T00:54:25Z

pkgs/development/python-modules/python-arango/default.nix

Then we should limit the package to those platforms or only the tests instead of disabling them completely.

I would like to do that, but I don't know how. How would you go about it?

@SuperSandro2000 Just for some background, the architectures that arango is sensitive to are not (i.e.) x86 vs aarch, it is haswell vs sandy-bridge.

Thats a problem because we have no system to detect this that does not require a mass rebuild. Can we force an older architecture like westmere?

pkgs/development/python-modules/python-arango/default.nix

jsoo1 · 2022-11-19T00:37:44Z

@SuperSandro2000 I reordered/renamed the attrs but I don't know how to conditionally disable the checks.

SuperSandro2000 · 2022-12-02T02:13:06Z

Ideally we want to force arango onto an older architecture like westmere.

Also please squash the review commits into the init one(s).

jsoo1 · 2022-12-02T02:18:13Z

Also please squash the review commits into the init one(s).

Squashed.

Ideally we want to force arango onto an older architecture like westmere.

Right now it defaults to haswell.

I think this client library shouldn't matter though, since it is going to be communicating with arangod over the network. Do you think it would be enough to add one commit removing the tests so they could be resurrected in a reverting commit if the arango architecture can be made portable?

Edit: I ask because it was a PITA to figure out the reasons tests were failing, arangod invocation for preCheck, etc.

SuperSandro2000 · 2022-12-03T01:48:01Z

We could make an option that disabled those tests. There are some other packages that do that already. Should have remembered that earlier.

jsoo1 · 2022-12-05T20:34:35Z

We could make an option that disabled those tests. There are some other packages that do that already. Should have remembered that earlier.

Could it be overridden in python-packages.nix?

SuperSandro2000 · 2022-12-05T21:27:47Z

For only tests that can be done.

SuperSandro2000 · 2022-12-06T00:54:56Z

Ah, sorry. I thought you had something else in mind other than disabling the tests by default. The only difference in your last commit is that we are now always using an overlay.

jsoo1 · 2022-12-06T00:56:58Z

Ah, sorry. I thought you had something else in mind other than disabling the tests by default. The only difference in your last commit is that we are now always using an overlay.

Ok right. Sorry I don't understand what you have in mind, then. Do you have an example maybe?

SuperSandro2000

we used enough time on this. Must be good enough for now without tests

jsoo1 · 2022-12-06T01:00:53Z

Ok. Thank you so much for your time!

SuperSandro2000 · 2022-12-06T01:23:01Z

@ofborg build python310Packages.python-arango

jsoo1 requested review from FRidh and jonringer as code owners September 30, 2022 22:46

github-actions bot added the 6.topic: python Python is a high-level, general-purpose programming language. label Sep 30, 2022

WolfangAukang suggested changes Oct 1, 2022

View reviewed changes

pkgs/development/python-modules/python-arango/default.nix Outdated Show resolved Hide resolved

pkgs/development/python-modules/python-arango/default.nix Outdated Show resolved Hide resolved

pkgs/development/python-modules/python-arango/default.nix Outdated Show resolved Hide resolved

jsoo1 changed the title ~~python3Packages/python-arango: init at 7.5.0~~ python3Packages.python-arango: init at 7.5.0 Oct 1, 2022

WolfangAukang suggested changes Oct 1, 2022

View reviewed changes

pkgs/development/python-modules/python-arango/default.nix Outdated Show resolved Hide resolved

jsoo1 changed the title ~~python3Packages.python-arango: init at 7.5.0~~ python3Packages.python-arango: init at 7.5.1 Oct 6, 2022

SuperSandro2000 reviewed Nov 17, 2022

View reviewed changes

python3Packages.python-arango: init at 7.5.3

09a033a

SuperSandro2000 approved these changes Dec 6, 2022

View reviewed changes

SuperSandro2000 merged commit 14c8855 into NixOS:master Dec 6, 2022

jsoo1 deleted the jsoo1/upstream-python-arango branch December 6, 2022 15:07

-  #
-  # nonetheless, the client library should remain in nixpkgs - since
-  # the client library will talk to arangodb across the network and
-  # architecture issues will be irrelevant.

Uh oh!

Conversation

jsoo1 commented Sep 30, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Description of changes

Things done

Uh oh!

WolfangAukang commented Oct 1, 2022

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

jsoo1 commented Oct 25, 2022

Uh oh!

jsoo1 commented Nov 8, 2022

Uh oh!

WolfangAukang commented Nov 8, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

jsoo1 commented Nov 9, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

WolfangAukang commented Nov 9, 2022

Uh oh!

jsoo1 commented Nov 9, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

WolfangAukang commented Nov 9, 2022

Uh oh!

jsoo1 commented Nov 11, 2022

Uh oh!

nixos-discourse commented Nov 16, 2022

Uh oh!

SuperSandro2000 left a comment

Choose a reason for hiding this comment

Uh oh!

SuperSandro2000 Nov 17, 2022

Choose a reason for hiding this comment

Uh oh!

SuperSandro2000 Nov 17, 2022

Choose a reason for hiding this comment

Uh oh!

jsoo1 Nov 19, 2022

Choose a reason for hiding this comment

Uh oh!

jsoo1 Dec 1, 2022

Choose a reason for hiding this comment

Uh oh!

SuperSandro2000 Dec 2, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

jsoo1 commented Nov 19, 2022

Uh oh!

SuperSandro2000 commented Dec 2, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

jsoo1 commented Dec 2, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

SuperSandro2000 commented Dec 3, 2022

Uh oh!

jsoo1 commented Dec 5, 2022

Uh oh!

SuperSandro2000 commented Dec 5, 2022

Uh oh!

SuperSandro2000 commented Dec 6, 2022

Uh oh!

jsoo1 commented Dec 6, 2022

Uh oh!

SuperSandro2000 left a comment

Choose a reason for hiding this comment

Uh oh!

jsoo1 commented Dec 6, 2022

Uh oh!

SuperSandro2000 commented Dec 6, 2022

Uh oh!

Reviewers

jsoo1 commented Sep 30, 2022 •

edited

Loading

WolfangAukang commented Nov 8, 2022 •

edited

Loading

jsoo1 commented Nov 9, 2022 •

edited

Loading

jsoo1 commented Nov 9, 2022 •

edited

Loading

SuperSandro2000 Dec 2, 2022 •

edited

Loading

SuperSandro2000 commented Dec 2, 2022 •

edited

Loading

jsoo1 commented Dec 2, 2022 •

edited

Loading