Skip to content

re-enabling integration tests for Sinks #12307

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom
Merged
lhotari merged 8 commits into from
Oct 14, 2021
Merged

re-enabling integration tests for Sinks #12307

lhotari merged 8 commits into from
Oct 14, 2021

Conversation

@dlg99
Copy link
Contributor

@dlg99 dlg99 commented Oct 8, 2021 •
edited
Loading

Motivation

Integration tests for sinks used to fail/timeout and were excluded when tests for Pulsar IO were brought back.

Modifications

Re-enabled tests.
Made sure that elasticClient is closed after the test (reduced flakiness locally)
Upgraded Elastic to a slightly newer version (w/smaller container).
That removed all flakiness on my mac.
CI still failed with random containers from pulsar failing to start, timing out etc (BK, broker)
Reduced memory for the Elastic container and it seems to fix the problems on CI.

There are still some tests with "enabled-false", I am not sure what's the history there and didn't look at these:

    @Test(enabled = false, groups = "sink")
    public void testCassandraSink() throws Exception {
..
    @Test(enabled = false, groups = "sink")
    public void testCassandraArchiveSink() throws Exception {
...
    @Test(enabled = false, groups = "sink")
    public void testHdfsSink() throws Exception {
...

Verifying this change

  • Make sure that the change passes the CI checks.

Does this pull request potentially affect one of the following parts:

No

Documentation

  • no-need-doc

@dlg99 dlg99 marked this pull request as draft October 9, 2021 02:00
@Anonymitaet Anonymitaet added the doc-not-needed Your PR changes do not impact docs label Oct 13, 2021
eolivelli
eolivelli approved these changes Oct 13, 2021
View reviewed changes
Copy link
Contributor

@eolivelli eolivelli left a comment

Choose a reason for hiding this comment

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

Very good

@vroyer you may be interested in this patch

@dlg99 dlg99 marked this pull request as ready for review October 13, 2021 21:00
nicoloboschi
nicoloboschi approved these changes Oct 14, 2021
View reviewed changes
Copy link
Contributor

@nicoloboschi nicoloboschi left a comment

Choose a reason for hiding this comment

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

great, +1

@eolivelli eolivelli added this to the 2.9.0 milestone Oct 14, 2021
lhotari
lhotari approved these changes Oct 14, 2021
View reviewed changes
Copy link
Member

@lhotari lhotari left a comment

Choose a reason for hiding this comment

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

LGTM

@lhotari lhotari merged commit 1553147 into apache:master Oct 14, 2021
zeo1995 pushed a commit to zeo1995/pulsar that referenced this pull request Oct 14, 2021
Merge branch 'up/master' into website/branch-2.7.2
82208c9 
* up/master: (37 commits)
  re-enabling integration tests for Sinks (apache#12307)
  [PIP 95][Issue 12040][web] Topic lookup with listener header (apache#12072)
  Fix the master CI broken with update dispatch rate block issue (apache#12360)
  Fix message being ignored when the non-persistent topic reader reconnect. (apache#12348)
  Fix log format. (apache#12346)
  [website][upgrade]feat: docs migration - version-2.7.2 Concepts and Architecture (apache#12354)
  [website][upgrade] feat: full docs migration for version 2.8.0 (apache#12359)
  [website][upgrade]feat: dynamic replace version info before build (apache#12337)
  Fix flaky tests: ElasticSearchClientTests (apache#12347)
  Use asyncCloseCursorLedger to replace cursorLedger.asyncClose method in the ManagedCursorImpl.VoidCallback#operationComplete (apache#12113)
  fix-npe-ZkBookieRackAffinityMapping (apache#11947)
  [pulsar-admin] Allow setting --forward-source-message-property to false when updating a pulsar function (apache#12128)
  [website][upgrade]feat: docs migration - Development (apache#12320)
  Update delete inactive topic configuration documentation (apache#12350)
  [PIP 95][Issue 12040][broker] Multiple bind addresses for Pulsar protocol (apache#12056)
  Added Debezium Source for MS SQL Server (apache#12256)
  Fix: flaky oracle tests (apache#12306)
  [C++] Use URL encoded content type for OAuth 2.0 authentication (apache#12341)
  [C++] Handle OAuth 2.0 exceptional cases gracefully (apache#12335)
  feat(cli): add restart command to pulsar-daemon (apache#12279)
  ...

# Conflicts:
#	site2/website-next/docusaurus.config.js
#	site2/website-next/versioned_sidebars/version-2.7.2-sidebars.json
#	site2/website-next/versions.json
@dlg99 dlg99 deleted the tests-sinks branch October 14, 2021 23:32
@lhotari lhotari mentioned this pull request Oct 30, 2021
Closed
bharanic-dev pushed a commit to bharanic-dev/pulsar that referenced this pull request Mar 18, 2022
@dlg99 @splunk-xingw
re-enabling integration tests for Sinks (apache#12307)
f983280 
* re-enabling integration tests for Sinks

* close elasticClient

* Upgrade Elastic container

* Split into two jobs, plus system prune

* disabling tests for elastic

* Revert "disabling tests for elastic"

This reverts commit 26862c3.

* reduce memory for elastic

* Revert "Split into two jobs, plus system prune"

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

Reviewers

@lhotari lhotari lhotari approved these changes

@eolivelli eolivelli eolivelli approved these changes

@nicoloboschi nicoloboschi nicoloboschi approved these changes

Assignees

No one assigned

Labels

doc-not-needed Your PR changes do not impact docs

Projects

None yet

Milestone

2.9.0

Development

Successfully merging this pull request may close these issues.

5 participants

@dlg99 @lhotari @eolivelli @nicoloboschi @Anonymitaet