Skip to content

Initializes Celery app in CeleryEmitter.#196

Closed
Julio Carlos Menendez (juliomenendez) wants to merge 1 commit into
snowplow:release/0.8.3from
rinse-inc:dont-initialize-celery-in-module
Closed

Initializes Celery app in CeleryEmitter.#196
Julio Carlos Menendez (juliomenendez) wants to merge 1 commit into
snowplow:release/0.8.3from
rinse-inc:dont-initialize-celery-in-module

Conversation

@juliomenendez

@juliomenendez Julio Carlos Menendez (juliomenendez) commented Mar 5, 2018

Copy link
Copy Markdown
Contributor

Fixes #185

@coveralls

Coveralls (coveralls) commented Mar 5, 2018

Copy link
Copy Markdown

Coverage Status

Coverage decreased (-0.6%) to 80.138% when pulling 0d87c2f on juliomenendez:dont-initialize-celery-in-module into 91da00a on snowplow:master.

@snowplowcla

Copy link
Copy Markdown

Thanks for your pull request. Is this your first contribution to a Snowplow open source project? Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA).

📝 Please visit https://github.com/snowplow/snowplow/wiki/CLA to learn more and sign.

Once you've signed, please reply here (e.g. I signed it!) and we'll verify. Thanks.

@juliomenendez

Copy link
Copy Markdown
Contributor Author

I signed it!

@BenFradet Ben Fradet (BenFradet) left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

LGTM, I'll let Anton Parkhomenko (@chuwy) have a second look

@chuwy Anton Parkhomenko (chuwy) left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Thank you so much Julio Carlos Menendez (@juliomenendez). That totally makes sense. I'd actually go further and make emiiters a package with separate celery_emitter module to not confuse people who don't use celery, but this is also a good solution for now.

@juliomenendez

Copy link
Copy Markdown
Contributor Author

Thanks! What should I do about the tests failing for Python 3.3? Remove that version entirely maybe?

@chuwy

Copy link
Copy Markdown
Contributor

Good question. For me Python 3.3 doesn't look outdated yet. Let's wait until we have a bandwidth for a new release and we'll decide what to do with 3.3 then.

Also, it looks like problem not in tracker, but in other our project - release-manager, which is a bit suspicious. Maybe we can stick with earlier version (e.g. 0.2.0)

@snowplowcla

Copy link
Copy Markdown

Confirmed! Julio Carlos Menendez (@juliomenendez) has signed the Individual Contributor License Agreement. Thanks so much

@mhadam
Michael Hadam (mhadam) changed the base branch from master to release/0.8.3 June 27, 2019 16:55
@oguzhanunlu

Copy link
Copy Markdown
Member

Hi Julio Carlos Menendez (@juliomenendez) , thanks for the contribution! It is cherry-picked to the release branch at #224 .

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants