python3Packages.aioinflux: init at 0.9.0 #70840

liamdiprose · 2019-10-09T08:23:12Z

Things done

jonringer · 2019-10-09T15:19:36Z

this has your other PR in it, I would just close the other one, and keep this one.

Also, can you please squash your commits so that they just related to the two packages, commit history should look like:

pythonPackages.ciso8601: init at <ver>
pythonPackages.aioinflux: init at 0.9.0

liamdiprose · 2019-10-09T22:28:05Z

Thanks for the pointers; I've squashed the commits for the two packages.

Hopefully everything works with rewritten history 😥

Do you still want me to rebase everything on origin/master?

jonringer · 2019-10-09T23:11:05Z

as long as there's no merge conflicts, rebasing isn't necessary

liamdiprose · 2019-10-09T23:15:21Z

I thought that would be the case, thought I'd just check anyway. I only learnt how to rebase 2 hours ago 😛

jonringer

nix-review passes on NixOS
diff LGTM

[6 built, 10 copied (176.9 MiB), 31.3 MiB DL]
https://github.com/NixOS/nixpkgs/pull/70840
3 package were build:
python27Packages.ciso8601 python37Packages.aioinflux python37Packages.ciso8601

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

jonringer · 2019-10-09T23:16:25Z

@GrahamcOfBorg build python27Packages.ciso8601 python37Packages.aioinflux python37Packages.ciso8601

jonringer · 2019-10-10T22:43:13Z

@GrahamcOfBorg build python27Packages.ciso8601 python37Packages.aioinflux python37Packages.ciso8601

risicle · 2019-10-10T22:43:16Z

Two things:

The tests for aioinflux don't actually run if you inspect the logs. I suspect their setup.py wasn't actually hooked up to a pytest test-runner. But when you set up a custom checkPhase to make them actually run you realize that they almost all use local networking and so won't work in the sandbox. So you may as well disable the tests and remove all those deps. 🤷‍♂️
You're pointing at the wrong github repo for aioinflux. It's actually at https://github.com/gusutabopb/aioinflux.

jonringer · 2019-10-10T22:48:33Z

good catches @risicle :)

liamdiprose · 2019-10-10T23:01:47Z

Very good catches indeed. I did notice the 0 tests ran, but have no idea why I thought nothing of it.

doCheck is now false, and the test dependencies have been removed.

Out of curiosity, would it be possible to do the network tests in the check stage? Something like the bittorrent test.

risicle · 2019-10-10T23:13:44Z

I don't know a lot about the nixos tests - @jonringer ?

jonringer · 2019-10-10T23:46:43Z

I honestly don't know either, but nixos tests allow for you to have many vm's on a virtual network, so they should in theory be able to do cross computer network communication. However, this is probably overkill for just the package, as it would also require nixos modules to configure the VMs initially.

For python packages, you will want to just run the unit tests, this should give a good indication that the package is valid with it's dependencies, without requiring too much setup or compute.

jonringer · 2019-11-12T02:25:59Z

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

+  };
+
+  propagatedBuildInputs = [ aiohttp pytz ciso8601 ]
+    ++ lib.optional pandasSupport pandas;


if it's highly recommended to use pandas, then I would just add this, otherwise leave out the logic to conditionally add. Python packages are weird and it's awkward if two packages have aioinflux as a dependency, then it get's added twice.

Pandas is one of the killer features of the package, so I'll remove the condition.

Does the double-add happen when one package has pandasSupport, while the other overrides/disables it? Differing buildInputs would mean differing hashes, so different install paths. I can see how that would be non-trivial to solve.

liamdiprose · 2019-11-12T02:48:49Z

This rebase is blowing up in my face. Not sure how other peoples commits are seeping in...

Solved, just needed to drop the commit. I'm guessing it'll get picked up in the merge later on.

jonringer · 2019-11-12T17:32:26Z

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

+  propagatedBuildInputs = [ aiohttp pytz ciso8601 pandas ];
+
+  # Tests require local networking so won't run in the sandbox
+  doCheck = false;


please try to do some tests, you can always scope tests using pytest -k 'not some_test_string'

Yes, I agree. I've looked upstream but haven't found any suitable test filters yet. I might contribute a few tests so we can verify the package requirements are right.

I understand @FRidh isn't in a hurry to merge any python packages until we solve the scalability problem. So this isn't a super high priority for me, but I am still interested in getting it merged at some point.

Just letting you know I haven't jumped ship!

it's certainly a salability issue for one person, but not as much of an issue for many

It's more solved than I thought then.
I wonder if we could use mypy for a minimal check for all python packages. It can find if there are missing dependencies.

looks like mypy package logic differs from standard python mechanisms and makes assumptions as to where the packages should be located. Probably not compatible with nix

https://mypy.readthedocs.io/en/latest/running_mypy.html#how-imports-are-found

Yeah, I've had trouble with the logic, and spent alot of time in that section of the mypy docs.

I guess nothing beats actually running the code in the interpreter, and any python package worth packaging will have good test coverage anyway.

you can also do pythonImportsCheck = [ "foo.bar" ]; to essentially run python -c 'import foo.bar'

I use it extensively

nixpkgs/pkgs/tools/admin/azure-cli/default.nix

Line 168 in 3140fa8

pythonImportsCheck = [

to ensure that the azure namespace isn't broken by a rogue __init__.py file

jonringer

sorry for the very long delay

jonringer · 2020-03-25T02:04:21Z

maintainers/maintainer-list.nix

+  liamdiprose = {
+    email = "liam@liamdiprose.com";
+    github = "liamdiprose";
+    githubId = 1769386;
+    name = "Liam Diprose";
+  };


this should be a separate commit

Roger roger. To be honest I thought I was the slacker 😛

Now that my country is locked down, I see no better option than to get back into open source. I'll be pushing soon.

jonringer

please resolve conflicts as well

stale · 2020-09-23T22:56:02Z

Hello, I'm a bot and I thank you in the name of the community for your contributions.

Nixpkgs is a busy repository, and unfortunately sometimes PRs get left behind for too long. Nevertheless, we'd like to help committers reach the PRs that are still important. This PR has had no activity for 180 days, and so I marked it as stale, but you can rest assured it will never be closed by a non-human.

If this is still important to you and you'd like to remove the stale label, we ask that you leave a comment. Your comment can be as simple as "still important to me". But there's a bit more you can do:

If you received an approval by an unprivileged maintainer and you are just waiting for a merge, you can @ mention someone with merge permissions and ask them to help. You might be able to find someone relevant by using Git blame on the relevant files, or via GitHub's web interface. You can see if someone's a member of the nixpkgs-committers team, by hovering with the mouse over their username on the web interface, or by searching them directly on the list.

If your PR wasn't reviewed at all, it might help to find someone who's perhaps a user of the package or module you are changing, or alternatively, ask once more for a review by the maintainer of the package/module this is about. If you don't know any, you can use Git blame on the relevant files, or GitHub's web interface to find someone who touched the relevant files in the past.

If your PR has had reviews and nevertheless got stale, make sure you've responded to all of the reviewer's requests / questions. Usually when PR authors show responsibility and dedication, reviewers (privileged or not) show dedication as well. If you've pushed a change, it's possible the reviewer wasn't notified about your push via email, so you can always officially request them for a review, or just @ mention them and say you've addressed their comments.

Lastly, you can always ask for help at our Discourse Forum, or more specifically, at this thread or at #nixos' IRC channel.

prusnak · 2020-12-03T18:55:18Z

@liamdiprose do you still want to get this in?

lopsided98 · 2021-02-04T00:41:08Z

I submitted an updated version of this PR: #111849

prusnak · 2021-02-04T08:31:33Z

Superseded by #111849

liamdiprose requested a review from FRidh as a code owner October 9, 2019 08:23

ofborg bot added 6.topic: python 8.has: package (new) 11.by: package-maintainer 10.rebuild-darwin: 1-10 10.rebuild-linux: 1-10 labels Oct 9, 2019

liamdiprose force-pushed the aioinflux-0.9.0 branch 4 times, most recently from 03ee334 to e748b0f Compare October 9, 2019 22:22

jonringer approved these changes Oct 9, 2019

View reviewed changes

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

liamdiprose force-pushed the aioinflux-0.9.0 branch 2 times, most recently from 3787dc2 to 85df678 Compare October 10, 2019 20:37

liamdiprose force-pushed the aioinflux-0.9.0 branch from 85df678 to 48ae291 Compare October 10, 2019 22:52

liamdiprose force-pushed the aioinflux-0.9.0 branch from 48ae291 to 64a4ee6 Compare November 11, 2019 22:05

jonringer reviewed Nov 12, 2019

View reviewed changes

liamdiprose force-pushed the aioinflux-0.9.0 branch 2 times, most recently from e31c1bf to 0e48fdb Compare November 12, 2019 02:47

Liam Diprose added 2 commits November 12, 2019 15:50

pythonPackages.ciso8601: init at 2.1.1

a540820

pythonPackages.aioinflux: init at 0.9.0

6d641f3

liamdiprose force-pushed the aioinflux-0.9.0 branch from 0e48fdb to 6d641f3 Compare November 12, 2019 02:50

jonringer reviewed Nov 12, 2019

View reviewed changes

jonringer reviewed Mar 25, 2020

View reviewed changes

jonringer reviewed Mar 27, 2020

View reviewed changes

stale bot added the 2.status: stale label Sep 23, 2020

ryantm added 2.status: merge conflict and removed 2.status: merge conflict labels Oct 3, 2020

stale bot removed the 2.status: stale label Oct 3, 2020

SuperSandro2000 marked this pull request as draft November 28, 2020 00:15

prusnak added the 2.status: stale label Dec 3, 2020

stale bot removed the 2.status: stale label Dec 3, 2020

lopsided98 mentioned this pull request Feb 4, 2021

pythonPackages.aioinflux: init at 0.9.0 #111849

Merged

10 tasks

prusnak closed this Feb 4, 2021

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

python3Packages.aioinflux: init at 0.9.0 #70840

python3Packages.aioinflux: init at 0.9.0 #70840

liamdiprose commented Oct 9, 2019 •

edited

Loading

jonringer commented Oct 9, 2019

liamdiprose commented Oct 9, 2019 •

edited

Loading

jonringer commented Oct 9, 2019

liamdiprose commented Oct 9, 2019

jonringer left a comment

jonringer commented Oct 9, 2019

jonringer commented Oct 10, 2019

risicle commented Oct 10, 2019

jonringer commented Oct 10, 2019

liamdiprose commented Oct 10, 2019

risicle commented Oct 10, 2019 •

edited

Loading

jonringer commented Oct 10, 2019

jonringer Nov 12, 2019

liamdiprose Nov 12, 2019 •

edited

Loading

liamdiprose commented Nov 12, 2019 •

edited

Loading

jonringer Nov 12, 2019

liamdiprose Dec 4, 2019

jonringer Dec 4, 2019

liamdiprose Dec 4, 2019

jonringer Dec 4, 2019

liamdiprose Dec 5, 2019

jonringer Dec 5, 2019 •

edited

Loading

jonringer left a comment

jonringer Mar 25, 2020

liamdiprose Mar 25, 2020

jonringer left a comment

stale bot commented Sep 23, 2020

prusnak commented Dec 3, 2020

lopsided98 commented Feb 4, 2021

prusnak commented Feb 4, 2021

python3Packages.aioinflux: init at 0.9.0 #70840

python3Packages.aioinflux: init at 0.9.0 #70840

Conversation

liamdiprose commented Oct 9, 2019 • edited Loading

Things done

jonringer commented Oct 9, 2019

liamdiprose commented Oct 9, 2019 • edited Loading

jonringer commented Oct 9, 2019

liamdiprose commented Oct 9, 2019

jonringer left a comment

Choose a reason for hiding this comment

jonringer commented Oct 9, 2019

jonringer commented Oct 10, 2019

risicle commented Oct 10, 2019

jonringer commented Oct 10, 2019

liamdiprose commented Oct 10, 2019

risicle commented Oct 10, 2019 • edited Loading

jonringer commented Oct 10, 2019

Choose a reason for hiding this comment

liamdiprose Nov 12, 2019 • edited Loading

Choose a reason for hiding this comment

liamdiprose commented Nov 12, 2019 • edited Loading

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

jonringer Dec 5, 2019 • edited Loading

Choose a reason for hiding this comment

jonringer left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jonringer left a comment

Choose a reason for hiding this comment

stale bot commented Sep 23, 2020

prusnak commented Dec 3, 2020

lopsided98 commented Feb 4, 2021

prusnak commented Feb 4, 2021

liamdiprose commented Oct 9, 2019 •

edited

Loading

liamdiprose commented Oct 9, 2019 •

edited

Loading

risicle commented Oct 10, 2019 •

edited

Loading

liamdiprose Nov 12, 2019 •

edited

Loading

liamdiprose commented Nov 12, 2019 •

edited

Loading

jonringer Dec 5, 2019 •

edited

Loading