Skip to content

Change default method to POST in emitter (close #289)#290

Merged
Jack Keene (Jack-Keene) merged 20 commits into
release/0.12.0from
issue/289_set_default_method_post
Nov 3, 2022
Merged

Change default method to POST in emitter (close #289)#290
Jack Keene (Jack-Keene) merged 20 commits into
release/0.12.0from
issue/289_set_default_method_post

Conversation

@Jack-Keene

Copy link
Copy Markdown
Contributor

This PR sets the default method to post in the emitter as well as setting the Default buffer size to 1 (see discussion in issue 289). It also updates the tests to reflect the expected changes.

@snowplowcla Snowplow CLA bot (snowplowcla) added the cla:no [Auto generated] Snowplow Contributor License Agreement has not been signed. label Nov 1, 2022
@Jack-Keene Jack Keene (Jack-Keene) removed the cla:no [Auto generated] Snowplow Contributor License Agreement has not been signed. label Nov 1, 2022

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.

Seems like there is an extra commit with changing the HTTPS default because we updated the release branch. You'll need to remove that from this branch, which is quite an annoying process, let me know if you need help.

Comment thread snowplow_tracker/emitters.py
Comment thread snowplow_tracker/emitters.py
@matus-tomlein Matus Tomlein (matus-tomlein) force-pushed the issue/289_set_default_method_post branch from 298e71c to 09ef976 Compare November 2, 2022 10:09

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.

One more thing that I just realized is that the example app makes use of an Emitter with default configuration – I suppose that won't track any events since there are less than 10 events tracked... Can you add a flush at the end?

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.

LGTM!

@matus-tomlein Matus Tomlein (matus-tomlein) changed the title Change default method to POST in emitter (Close #289) Change default method to POST in emitter (close #289) Nov 3, 2022
@Jack-Keene Jack Keene (Jack-Keene) merged commit 2cd6abb into release/0.12.0 Nov 3, 2022
@Jack-Keene Jack Keene (Jack-Keene) deleted the issue/289_set_default_method_post branch November 3, 2022 10:06
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.

3 participants