Add pykafka recipe #4091

mrocklin · 2017-10-01T13:40:44Z

cc @emmett9001

Depends on #4090

conda-forge-linter · 2017-10-01T13:40:51Z

Hi! This is the friendly automated conda-forge-linting service.

I just wanted to let you know that I linted all conda-recipes in your PR (recipes/pykafka) and found it was in an excellent condition.

conda-forge-linter · 2017-10-01T13:57:42Z

Hi! This is the friendly automated conda-forge-linting service.

I wanted to let you know that I linted all conda-recipes in your PR (recipes/pykafka) and found some lint.

Here's what I've got...

For recipes/pykafka:

Selectors are suggested to take a <two spaces>#<one space>[<expression>] form.

conda-forge-linter · 2017-10-01T14:00:21Z

Hi! This is the friendly automated conda-forge-linting service.

I just wanted to let you know that I linted all conda-recipes in your PR (recipes/pykafka) and found it was in an excellent condition.

mrocklin · 2017-10-01T21:13:59Z

@emmett9001 I found that I needed to install gevent in order to import pykafka. Is this expected?

jakirkham · 2017-10-01T21:37:43Z

recipes/pykafka/meta.yaml

+extra:
+  recipe-maintainers:
+    - mrocklin
+    - emmett9001


Are you ok being a maintainer, @emmett9001?

jakirkham · 2017-10-01T21:38:33Z

Just want to check with the other person to make sure they are ok maintaining this. Otherwise LGTM.

mrocklin · 2017-10-01T23:51:19Z

Just want to check with the other person to make sure they are ok maintaining this. Otherwise LGTM.

Yes, we're in conversation in Parsely/pykafka#730 . I haven't yet received a confirmation.

Also, what is the right thing to do if the package needs pkg_resources at runtime? Add a runtime dependency on setuptools?

mrocklin · 2017-10-02T01:00:35Z

It looks like we're not picking up the librdkafka library at build time. http://pykafka.readthedocs.io/en/latest/#using-the-librdkafka-extension

Is there a nice way to add conda's local include and lib directories to C_INCLUDE_PATH and LD_LIBRARY_PATH? Is this typically done in a build.sh script or is there some more standard approach?

jakirkham · 2017-10-02T01:03:06Z

recipes/pykafka/meta.yaml

+
+requirements:
+  build:
+    - python


Please add toolchain.

jakirkham · 2017-10-02T01:03:43Z

Also, what is the right thing to do if the package needs pkg_resources at runtime? Add a runtime dependency on setuptools?

Yes.

Is there a nice way to add conda's local include and lib directories to C_INCLUDE_PATH and LD_LIBRARY_PATH? Is this typically done in a build.sh script or is there some more standard approach?

Adding toolchain above should fix it.

mrocklin · 2017-10-02T01:09:43Z

This recipe has grown somewhat more complex. Does it still meet the requirements of noarch: python?

mrocklin · 2017-10-02T01:10:39Z

It looks like we're still not able to see the librdkafka library. I'm not sure if this is at build time or at runtime though.

mrocklin · 2017-10-02T01:27:11Z

Failing at build time

pykafka/rdkafka/_rd_kafkamodule.c:11:32: fatal error: librdkafka/rdkafka.h: No such file or directory

jakirkham · 2017-10-02T01:35:25Z

Does it still meet the requirements of noarch: python?

If it is compiling stuff, would say no.

jakirkham · 2017-10-02T01:36:25Z

recipes/pykafka/meta.yaml

+  build:
+    - python
+    - setuptools
+    - toolchain


Please add librdkafka here too if it is needed for the build.

Ah ah! Of course. Now I feel a bit silly. Thanks @jakirkham !

jakirkham · 2017-10-02T01:49:12Z

recipes/pykafka/meta.yaml

+    - python
+    - setuptools
+    - toolchain
+    - librdkafka  # [not win]


Normally for libraries that are linked to, we pick some sort of pinning so the stack remains consistent and things don't break when a new version comes out. Based on the API/ABI report, it looks like librdkafka breaks on patch versions sometimes. Though 0.9.4 and 0.9.5 seem ok. Given this, it seems reasonable to change this to librdkafka 0.9.4|0.9.5. Thoughts on this?

cc @conda-forge/librdkafka

👍 I'm pinned at 0.9.4 here; this works for me.

I'd argue it's important to keep all of the libraries that depend on librdfakfa on the same pin so they are interchangable. Is there a non-manual way to do this other than a global conda-forge pin?

https://github.com/conda-forge/python-confluent-kafka-feedstock

https://github.com/conda-forge/kafkacat-feedstock

Hrm, at runtime I would expect conda itself to figure this out. I'm not sure how feasible it is to use pykafka with a different librdkafka from which it was built.

emmettbutler · 2017-10-02T16:37:27Z

@mrocklin gevent is still required as of 2.6.0, but 2.7.0.dev1 removes the dependency.

emmettbutler · 2017-10-02T16:39:52Z

How does conda-forge handle development versions? I've been doing pre-release versions for a while (with .dev on the end of the version number), and I know setuptools automatically ignores these versions during pip install. Moving forward, should I also do conda-forge pre-release versions this way?

emmettbutler · 2017-10-02T16:42:18Z

recipes/pykafka/meta.yaml

@@ -0,0 +1,60 @@
+{% set name = "pykafka" %}
+{% set version = "2.6.0" %}
+{% set sha256 = "19e4a802b00e28c3203fcfd80ca440b13faecb8e58171e9685a044cf87dae93e" %}


Where on PyPI is this SHA available?

https://pypi.org/project/pykafka/#files

mrocklin · 2017-10-02T17:03:39Z

How does conda-forge handle development versions? I've been doing pre-release versions for a while (with .dev on the end of the version number), and I know setuptools automatically ignores these versions during pip install. Moving forward, should I also do conda-forge pre-release versions this way?

I honestly never do dev releases on conda-forge. But then again I rarely do dev releases. I don't have much to provide here. @jakirkham generally knows best practices.

@mrocklin gevent is still required as of 2.6.0, but 2.7.0.dev1 removes the dependency.

OK, good to know.

From my perspective this PR is good to go.

mrocklin · 2017-10-04T12:30:58Z

Is there anything else here that needs to be resolved?

mrocklin · 2017-10-08T12:39:20Z

Ping

jakirkham · 2017-10-08T18:39:25Z

recipes/pykafka/meta.yaml

+    - python
+    - setuptools
+    - toolchain
+    - librdkafka 0.9.4|0.9.5  # [not win]


Could you please add this pinning to our list of pinnings?

jakirkham · 2017-10-08T18:41:38Z

Sorry @mrocklin. Got a bit swamped.

Would like us to add the librdkafka pinning to our list of pinnings. Otherwise this should be good to go.

mrocklin · 2017-10-08T18:45:11Z

Would like us to add the librdkafka pinning to our list of pinnings. Otherwise this should be good to go.

I'm not sure I understand the intent here or the format of that document. I'm happy to submit a PR that follows that pattern though. What happens if we update librdkafka on conda-forge (which is a few versions old at the moment).

jakirkham · 2017-10-08T20:26:43Z

So we try to keep all pinnings the same throughout conda-forge. That way, one can confidently install and use various packages from conda-forge without running into dependency conflicts.

ATM we pin build and run the same (though that may change in the future). The package name is listed on the left and on the right is the pinning that we are using in the org. The comment is there to note what defaults is pinning a package to (if known or if it exists in defaults).

Hope that helps. If you have more questions, please feel free to ask. :)

mrocklin · 2017-10-08T22:00:33Z

Ah, so all conda-forge recipes are pinned to the exact same version? How/when does the community decide to bump these pinned versions?

As requested in: conda-forge/staged-recipes#4091 (comment)

jakirkham · 2017-10-09T00:34:11Z

Yeah we try our best to do so. Though we haven't equipped the linter to enforce this and some recipes lag (though the pinning bot should help here). So there could be some exceptions. That said, the goal is to keep everything in lockstep.

Normally someone in the community feels a need to have a newer version and they PR a version bump to the webpage repo. Then we try to discuss with relevant parties to see if that works for everyone affected. Though we may go ahead if some people are slow to reply.

jakirkham · 2017-10-09T22:50:28Z

Thanks @mrocklin.

mrocklin · 2017-10-09T22:58:32Z

Thank you for shepherding this along @jakirkham

mrocklin force-pushed the pykafka branch from c051a93 to dc22eed Compare October 1, 2017 13:43

mrocklin mentioned this pull request Oct 1, 2017

Add pykafka recipe to conda-forge Parsely/pykafka#730

Closed

mrocklin force-pushed the pykafka branch from dc22eed to f4023b7 Compare October 1, 2017 13:57

mrocklin force-pushed the pykafka branch from f4023b7 to c673e31 Compare October 1, 2017 14:00

mrocklin force-pushed the pykafka branch from c673e31 to 05e635e Compare October 1, 2017 14:45

Add pykafka recipe

740e326

mrocklin force-pushed the pykafka branch from 05e635e to 740e326 Compare October 1, 2017 20:35

add gevent to dependencies

859ec1c

jakirkham reviewed Oct 1, 2017

View reviewed changes

test rdkafka import

739e224

add setuptools

4b2a927

jakirkham reviewed Oct 2, 2017

View reviewed changes

recipes/pykafka/meta.yaml

requirements:

build:

- python

Copy link

Member

jakirkham Oct 2, 2017

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please add toolchain.

add toolchain to build requirements

f1d526d

jakirkham reviewed Oct 2, 2017

View reviewed changes

add librdkafka to build, remove noarch

3fba27c

jakirkham reviewed Oct 2, 2017

View reviewed changes

pin librdkafka

6723757

emmettbutler reviewed Oct 2, 2017

View reviewed changes

jakirkham reviewed Oct 8, 2017

View reviewed changes

mrocklin added a commit to mrocklin/conda-forge.github.io that referenced this pull request Oct 8, 2017

Pin librdkafka

8048439

As requested in: conda-forge/staged-recipes#4091 (comment)

mrocklin mentioned this pull request Oct 8, 2017

Pin librdkafka conda-forge/conda-forge.github.io#463

Merged

jakirkham merged commit 523caef into conda-forge:master Oct 9, 2017

mrocklin deleted the pykafka branch October 9, 2017 22:58

Add pykafka recipe #4091

Add pykafka recipe #4091

Conversation

mrocklin commented Oct 1, 2017

conda-forge-linter commented Oct 1, 2017

conda-forge-linter commented Oct 1, 2017

conda-forge-linter commented Oct 1, 2017

mrocklin commented Oct 1, 2017

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jakirkham commented Oct 1, 2017

mrocklin commented Oct 1, 2017

mrocklin commented Oct 2, 2017

Choose a reason for hiding this comment

jakirkham commented Oct 2, 2017

mrocklin commented Oct 2, 2017

mrocklin commented Oct 2, 2017

mrocklin commented Oct 2, 2017

jakirkham commented Oct 2, 2017

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

emmettbutler commented Oct 2, 2017

emmettbutler commented Oct 2, 2017

Choose a reason for hiding this comment

Choose a reason for hiding this comment

mrocklin commented Oct 2, 2017

mrocklin commented Oct 4, 2017

mrocklin commented Oct 8, 2017

Choose a reason for hiding this comment

jakirkham commented Oct 8, 2017

mrocklin commented Oct 8, 2017

jakirkham commented Oct 8, 2017

mrocklin commented Oct 8, 2017

jakirkham commented Oct 9, 2017

jakirkham commented Oct 9, 2017

mrocklin commented Oct 9, 2017