Skip to content
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

Apply setting subscriber's partition to empty set [12317] #2108

Merged
merged 5 commits into from
Aug 6, 2021
Merged

Apply setting subscriber's partition to empty set [12317] #2108

merged 5 commits into from
Aug 6, 2021

Conversation

EduPonz
Copy link

@EduPonz EduPonz commented Aug 2, 2021 •
edited
Loading

Fixes #2107

  • Fix
  • Regression tests

Signed-off-by: Eduardo Ponz Segrelles eduardoponz@eprosima.com

@EduPonz
Refs 12317: Apply setting subscriber's partition to empty set
727c839 
Signed-off-by: Eduardo Ponz Segrelles <eduardoponz@eprosima.com>
@EduPonz EduPonz added this to the v2.4.0 milestone Aug 2, 2021
@EduPonz EduPonz requested a review from MiguelCompany August 2, 2021 12:06
@EduPonz EduPonz mentioned this pull request Aug 2, 2021
Closed
EduPonz added 2 commits August 2, 2021 21:28
@EduPonz
Refs 12317: Test for checking that partition QoS are updated with Sub…
8381d3c 
…scriber::set_qos

Signed-off-by: Eduardo Ponz Segrelles <eduardoponz@eprosima.com>
@EduPonz
Refs 12317: Blackbox test for clearing a non-empty set of partitions …
c87d60d 
…in subscriber

Signed-off-by: Eduardo Ponz Segrelles <eduardoponz@eprosima.com>
@EduPonz EduPonz marked this pull request as ready for review August 2, 2021 19:52
@EduPonz EduPonz changed the title Apply setting subscriber's partition to empty set Apply setting subscriber's partition to empty set [12317] Aug 3, 2021
MiguelCompany
MiguelCompany previously approved these changes Aug 3, 2021
View reviewed changes
Copy link
Member

@MiguelCompany MiguelCompany left a comment

Choose a reason for hiding this comment

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

LGTM

@EduPonz
Refs 12317: Clarify doxygen
27fd0af 
Signed-off-by: Eduardo Ponz Segrelles <eduardoponz@eprosima.com>
@EduPonz EduPonz force-pushed the bugfix/update_empty_sub_partitions branch from ee275e9 to 27fd0af Compare August 3, 2021 05:21
MiguelCompany
MiguelCompany approved these changes Aug 3, 2021
View reviewed changes
Copy link
Member

@MiguelCompany MiguelCompany left a comment

Choose a reason for hiding this comment

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

LGTM

MiguelCompany
MiguelCompany requested changes Aug 3, 2021
View reviewed changes
Copy link
Member

@MiguelCompany MiguelCompany left a comment

Choose a reason for hiding this comment

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

  • Fix warning on windows
  • Discovery server test_34_connect_locally_with_remote_server failure is unusual

@EduPonz
Refs 12317: Address Windows warning
80f5f1c 
Signed-off-by: Eduardo Ponz Segrelles <eduardoponz@eprosima.com>
@EduPonz
Copy link
Author

EduPonz commented Aug 3, 2021 •
edited
Loading

  • I think I've addressed the Windows warnings in 80f5f1c.
  • Failure of test_34_connect_locally_with_remote_server seems totally unrelated. Additionally, in the CI run for 727c839 the test passed, as well as locally for me

@MiguelCompany MiguelCompany added the ready-to-merge Ready to be merged. CI and changes have been reviewed and approved. label Aug 4, 2021
MiguelCompany
MiguelCompany approved these changes Aug 4, 2021
View reviewed changes
Copy link
Member

@MiguelCompany MiguelCompany left a comment

Choose a reason for hiding this comment

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

LGTM, but CI should be re-run after merging #1996

@EduPonz
Copy link
Author

EduPonz commented Aug 4, 2021

@Mergifyio rebase 2.0.x 2.1.x 2.2.x 2.3.x

@mergify
Copy link
Contributor

mergify bot commented Aug 4, 2021

Command rebase 2.0.x 2.1.x 2.2.x 2.3.x: success

Branch already up to date

@EduPonz
Copy link
Author

EduPonz commented Aug 4, 2021

@Mergifyio backport 2.0.x 2.1.x 2.2.x 2.3.x

@mergify
Copy link
Contributor

mergify bot commented Aug 4, 2021

Command backport 2.0.x 2.1.x 2.2.x 2.3.x: pending

Waiting for the pull request to get merged

@MiguelCompany
Copy link
Member

@richiprosima Please test this

@MiguelCompany MiguelCompany merged commit 331f1ca into master Aug 6, 2021
@MiguelCompany MiguelCompany deleted the bugfix/update_empty_sub_partitions branch August 6, 2021 07:18
mergify bot pushed a commit that referenced this pull request Aug 6, 2021
@EduPonz
Apply setting subscriber's partition to empty set (#2108)
8313d3a 
* Refs 12317: Apply setting subscriber's partition to empty set

Signed-off-by: Eduardo Ponz Segrelles <eduardoponz@eprosima.com>

* Refs 12317: Test for checking that partition QoS are updated with Subscriber::set_qos

Signed-off-by: Eduardo Ponz Segrelles <eduardoponz@eprosima.com>

* Refs 12317: Blackbox test for clearing a non-empty set of partitions in subscriber

Signed-off-by: Eduardo Ponz Segrelles <eduardoponz@eprosima.com>

* Refs 12317: Clarify doxygen

Signed-off-by: Eduardo Ponz Segrelles <eduardoponz@eprosima.com>

* Refs 12317: Address Windows warning

Signed-off-by: Eduardo Ponz Segrelles <eduardoponz@eprosima.com>
(cherry picked from commit 331f1ca)
@mergify mergify bot mentioned this pull request Aug 6, 2021
Merged
mergify bot pushed a commit that referenced this pull request Aug 6, 2021
@EduPonz
Apply setting subscriber's partition to empty set (#2108)
10153e8 
* Refs 12317: Apply setting subscriber's partition to empty set

Signed-off-by: Eduardo Ponz Segrelles <eduardoponz@eprosima.com>

* Refs 12317: Test for checking that partition QoS are updated with Subscriber::set_qos

Signed-off-by: Eduardo Ponz Segrelles <eduardoponz@eprosima.com>

* Refs 12317: Blackbox test for clearing a non-empty set of partitions in subscriber

Signed-off-by: Eduardo Ponz Segrelles <eduardoponz@eprosima.com>

* Refs 12317: Clarify doxygen

Signed-off-by: Eduardo Ponz Segrelles <eduardoponz@eprosima.com>

* Refs 12317: Address Windows warning

Signed-off-by: Eduardo Ponz Segrelles <eduardoponz@eprosima.com>
(cherry picked from commit 331f1ca)

# Conflicts:
#	test/unittest/dds/subscriber/SubscriberTests.cpp
@mergify mergify bot mentioned this pull request Aug 6, 2021
Merged
mergify bot pushed a commit that referenced this pull request Aug 6, 2021
@EduPonz
Apply setting subscriber's partition to empty set (#2108)
5e4f2ca 
* Refs 12317: Apply setting subscriber's partition to empty set

Signed-off-by: Eduardo Ponz Segrelles <eduardoponz@eprosima.com>

* Refs 12317: Test for checking that partition QoS are updated with Subscriber::set_qos

Signed-off-by: Eduardo Ponz Segrelles <eduardoponz@eprosima.com>

* Refs 12317: Blackbox test for clearing a non-empty set of partitions in subscriber

Signed-off-by: Eduardo Ponz Segrelles <eduardoponz@eprosima.com>

* Refs 12317: Clarify doxygen

Signed-off-by: Eduardo Ponz Segrelles <eduardoponz@eprosima.com>

* Refs 12317: Address Windows warning

Signed-off-by: Eduardo Ponz Segrelles <eduardoponz@eprosima.com>
(cherry picked from commit 331f1ca)
@mergify mergify bot mentioned this pull request Aug 6, 2021
Merged
mergify bot pushed a commit that referenced this pull request Aug 6, 2021
@EduPonz
Apply setting subscriber's partition to empty set (#2108)
a1ef2ba 
* Refs 12317: Apply setting subscriber's partition to empty set

Signed-off-by: Eduardo Ponz Segrelles <eduardoponz@eprosima.com>

* Refs 12317: Test for checking that partition QoS are updated with Subscriber::set_qos

Signed-off-by: Eduardo Ponz Segrelles <eduardoponz@eprosima.com>

* Refs 12317: Blackbox test for clearing a non-empty set of partitions in subscriber

Signed-off-by: Eduardo Ponz Segrelles <eduardoponz@eprosima.com>

* Refs 12317: Clarify doxygen

Signed-off-by: Eduardo Ponz Segrelles <eduardoponz@eprosima.com>

* Refs 12317: Address Windows warning

Signed-off-by: Eduardo Ponz Segrelles <eduardoponz@eprosima.com>
(cherry picked from commit 331f1ca)
@mergify mergify bot mentioned this pull request Aug 6, 2021
Merged
@mergify
Copy link
Contributor

mergify bot commented Aug 6, 2021

Command backport 2.0.x 2.1.x 2.2.x 2.3.x: success

Backports have been created

EduPonz added a commit that referenced this pull request Aug 13, 2021
@mergify @EduPonz
Apply setting subscriber's partition to empty set (#2108) (#2126)
9fdf2b4 
* Refs 12317: Apply setting subscriber's partition to empty set

Signed-off-by: Eduardo Ponz Segrelles <eduardoponz@eprosima.com>

* Refs 12317: Test for checking that partition QoS are updated with Subscriber::set_qos

Signed-off-by: Eduardo Ponz Segrelles <eduardoponz@eprosima.com>

* Refs 12317: Blackbox test for clearing a non-empty set of partitions in subscriber

Signed-off-by: Eduardo Ponz Segrelles <eduardoponz@eprosima.com>

* Refs 12317: Clarify doxygen

Signed-off-by: Eduardo Ponz Segrelles <eduardoponz@eprosima.com>

* Refs 12317: Address Windows warning

Signed-off-by: Eduardo Ponz Segrelles <eduardoponz@eprosima.com>
(cherry picked from commit 331f1ca)

Co-authored-by: Eduardo Ponz Segrelles <eduardoponz@eprosima.com>
SamuelWHalodi pushed a commit to SamuelWHalodi/Fast-DDS that referenced this pull request Oct 4, 2021
@EduPonz @SamuelWHalodi
Apply setting subscriber's partition to empty set (eProsima#2108)
783a4ec 
* Refs 12317: Apply setting subscriber's partition to empty set

Signed-off-by: Eduardo Ponz Segrelles <eduardoponz@eprosima.com>

* Refs 12317: Test for checking that partition QoS are updated with Subscriber::set_qos

Signed-off-by: Eduardo Ponz Segrelles <eduardoponz@eprosima.com>

* Refs 12317: Blackbox test for clearing a non-empty set of partitions in subscriber

Signed-off-by: Eduardo Ponz Segrelles <eduardoponz@eprosima.com>

* Refs 12317: Clarify doxygen

Signed-off-by: Eduardo Ponz Segrelles <eduardoponz@eprosima.com>

* Refs 12317: Address Windows warning

Signed-off-by: Eduardo Ponz Segrelles <eduardoponz@eprosima.com>
Signed-off-by: Samuel Wilhelmsson <samuel@halodi.com>
MiguelCompany added a commit that referenced this pull request Oct 15, 2021
@mergify @EduPonz @MiguelCompany
Apply setting subscriber's partition to empty set (#2125)
12d7385 
* Apply setting subscriber's partition to empty set (#2108)

* Refs 12317: Apply setting subscriber's partition to empty set

Signed-off-by: Eduardo Ponz Segrelles <eduardoponz@eprosima.com>

* Refs 12317: Test for checking that partition QoS are updated with Subscriber::set_qos

Signed-off-by: Eduardo Ponz Segrelles <eduardoponz@eprosima.com>

* Refs 12317: Blackbox test for clearing a non-empty set of partitions in subscriber

Signed-off-by: Eduardo Ponz Segrelles <eduardoponz@eprosima.com>

* Refs 12317: Clarify doxygen

Signed-off-by: Eduardo Ponz Segrelles <eduardoponz@eprosima.com>

* Refs 12317: Address Windows warning

Signed-off-by: Eduardo Ponz Segrelles <eduardoponz@eprosima.com>
(cherry picked from commit 331f1ca)

# Conflicts:
#	test/unittest/dds/subscriber/SubscriberTests.cpp

* Fix conflicts

Signed-off-by: Miguel Company <MiguelCompany@eprosima.com>

* Uncrustify

Signed-off-by: Miguel Company <MiguelCompany@eprosima.com>

* Removed unrelated test.

Signed-off-by: Miguel Company <MiguelCompany@eprosima.com>

Co-authored-by: Eduardo Ponz Segrelles <eduardoponz@eprosima.com>
Co-authored-by: Miguel Company <miguelcompany@eprosima.com>
MiguelCompany pushed a commit that referenced this pull request Feb 22, 2022
@mergify @EduPonz @JLBuenoLopez
Apply setting subscriber's partition to empty set (#2124)
7b4f6b7 
* Apply setting subscriber's partition to empty set (#2108)

* Refs 12317: Apply setting subscriber's partition to empty set

Signed-off-by: Eduardo Ponz Segrelles <eduardoponz@eprosima.com>

* Refs 12317: Test for checking that partition QoS are updated with Subscriber::set_qos

Signed-off-by: Eduardo Ponz Segrelles <eduardoponz@eprosima.com>

* Refs 12317: Blackbox test for clearing a non-empty set of partitions in subscriber

Signed-off-by: Eduardo Ponz Segrelles <eduardoponz@eprosima.com>

* Refs 12317: Clarify doxygen

Signed-off-by: Eduardo Ponz Segrelles <eduardoponz@eprosima.com>

* Refs 12317: Address Windows warning

Signed-off-by: Eduardo Ponz Segrelles <eduardoponz@eprosima.com>
(cherry picked from commit 331f1ca)

* Refs #12317: linters

Signed-off-by: JLBuenoLopez-eProsima <joseluisbueno@eprosima.com>

Co-authored-by: Eduardo Ponz Segrelles <eduardoponz@eprosima.com>
Co-authored-by: JLBuenoLopez-eProsima <joseluisbueno@eprosima.com>
@JLBuenoLopez JLBuenoLopez mentioned this pull request Jun 20, 2023
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@MiguelCompany MiguelCompany MiguelCompany approved these changes

Assignees
No one assigned
Labels
ready-to-merge Ready to be merged. CI and changes have been reviewed and approved.
Projects
None yet
Milestone
v2.4.0
Development

Successfully merging this pull request may close these issues.

Changing Partition QoS on running Publisher and Subscriber has no affect [12315]
2 participants
@EduPonz @MiguelCompany