[jira] [Commented] (MESOS-6181) The logic for BadACLNoPrincipal and BadACLDropCreateAndDestroy is not correct
[
https://issues.apache.org/jira/browse/MESOS-6181?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=15543225#comment-15543225
]
Greg Mann commented on MESOS-6181:
--
[~gyliu] yes, I see your point! Sorry :) I left a comment on the RR. Thanks!!
> The logic for BadACLNoPrincipal and BadACLDropCreateAndDestroy is not correct
> -
>
> Key: MESOS-6181
> URL: https://issues.apache.org/jira/browse/MESOS-6181
> Project: Mesos
> Issue Type: Bug
>Reporter: Guangya Liu
>Assignee: Guangya Liu
>
> Two issues for those two test cases:
> 1) No need to add `{}` in the test case as there is no need to add `{}`,
> adding the `{}` will cause the driver decline a non exist offer.
> 2) If destroy volume failed, we should get the last offer to make sure that
> the last offer also contain the volume resource.
--
This message was sent by Atlassian JIRA
(v6.3.4#6332)
[jira] [Commented] (MESOS-6181) The logic for BadACLNoPrincipal and BadACLDropCreateAndDestroy is not correct
[
https://issues.apache.org/jira/browse/MESOS-6181?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=15525325#comment-15525325
]
Guangya Liu commented on MESOS-6181:
Thanks [~greggomann] Agree for #1.
For #2, take {{PersistentVolumeTest, BadACLNoPrincipal}} as an example, in
https://github.com/apache/mesos/blob/master/src/tests/persistent_volume_tests.cpp#L1626
, it is expecting
{{EXPECT_TRUE(Resources(offer.resources()).contains(volume));}} , but it is not
using the latest offer but it is still using the offer
https://github.com/apache/mesos/blob/master/src/tests/persistent_volume_tests.cpp#L1599
revived, this is not accurate, we should use the offer after {{acceptOffers}}
, we need to make sure that the volume is still in the new offer after
allocation interval, comments?
> The logic for BadACLNoPrincipal and BadACLDropCreateAndDestroy is not correct
> -
>
> Key: MESOS-6181
> URL: https://issues.apache.org/jira/browse/MESOS-6181
> Project: Mesos
> Issue Type: Bug
>Reporter: Guangya Liu
>Assignee: Guangya Liu
>
> Two issues for those two test cases:
> 1) No need to add `{}` in the test case as there is no need to add `{}`,
> adding the `{}` will cause the driver decline a non exist offer.
> 2) If destroy volume failed, we should get the last offer to make sure that
> the last offer also contain the volume resource.
--
This message was sent by Atlassian JIRA
(v6.3.4#6332)
[jira] [Commented] (MESOS-6181) The logic for BadACLNoPrincipal and BadACLDropCreateAndDestroy is not correct
[
https://issues.apache.org/jira/browse/MESOS-6181?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=15524145#comment-15524145
]
Greg Mann commented on MESOS-6181:
--
[~gyliu], sorry for my delayed reply! Regarding the use of `{` and `}` to
enclose sections of the test code, I like to do this purely for readability.
i.e., instead of re-assigning the value of {{volume}} several times in a test,
it can be nice to declare the variable each time, enclosing it in a scope to
make it obvious where that volume is used, and where it goes out of scope. If
we use a single variable and re-assign its value multiple times in the test
without using scopes, I think it's more difficult to tell what the variable
contains at a given point in time.
However, this particular case isn't a very compelling use of this pattern, I'd
say. Unfortunately, we can't enclose the second volume-creation in a scope,
since we reference that volume at the end of the test when we call
{{EXPECT_TRUE(Resources(offer.resources()).contains(volume));}}.
Nonetheless, I believe the logic in the test is correct. Regarding your point
#1, the driver doesn't decline a non-existent offer, since the {{offer}}
variable is declared outside of the `{}` scope, it can be assigned a new value
within the scope and that value will still be accessible after the scope is
exited. Regarding point #2, the tests do actually test to make sure the final
offer contains the volume, using
{{EXPECT_TRUE(Resources(offer.resources()).contains(volume));}}.
What do you think?
> The logic for BadACLNoPrincipal and BadACLDropCreateAndDestroy is not correct
> -
>
> Key: MESOS-6181
> URL: https://issues.apache.org/jira/browse/MESOS-6181
> Project: Mesos
> Issue Type: Bug
>Reporter: Guangya Liu
>Assignee: Guangya Liu
>
> Two issues for those two test cases:
> 1) No need to add `{}` in the test case as there is no need to add `{}`,
> adding the `{}` will cause the driver decline a non exist offer.
> 2) If destroy volume failed, we should get the last offer to make sure that
> the last offer also contain the volume resource.
--
This message was sent by Atlassian JIRA
(v6.3.4#6332)
[jira] [Commented] (MESOS-6181) The logic for BadACLNoPrincipal and BadACLDropCreateAndDestroy is not correct
[
https://issues.apache.org/jira/browse/MESOS-6181?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=15497632#comment-15497632
]
Guangya Liu commented on MESOS-6181:
cc [~greggomann]
> The logic for BadACLNoPrincipal and BadACLDropCreateAndDestroy is not correct
> -
>
> Key: MESOS-6181
> URL: https://issues.apache.org/jira/browse/MESOS-6181
> Project: Mesos
> Issue Type: Bug
>Reporter: Guangya Liu
>Assignee: Guangya Liu
>
> Two issues for those two test cases:
> 1) No need to add `{}` in the test case as there is no need to add `{}`,
> adding the `{}` will cause the driver decline a non exist offer.
> 2) If destroy volume failed, we should get the last offer to make sure that
> the last offer also contain the volume resource.
--
This message was sent by Atlassian JIRA
(v6.3.4#6332)
