Skip to content
This repository has been archived by the owner on Aug 2, 2022. It is now read-only.

Fix update monitor from moniotr list #64

Merged
merged 1 commit into from
Jun 26, 2019
Merged

Fix update monitor from moniotr list #64

merged 1 commit into from
Jun 26, 2019

Conversation

mihirsoni
Copy link
Contributor

@mihirsoni mihirsoni commented Jun 26, 2019

Enable / Disable breaks from Monitor list.

Description of changes: Pass SeqNo and PrimaryTerm from actions.

Need port back to 1.0 branch.

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.

@mihirsoni mihirsoni requested review from dbbaughe and ylwu-amzn June 26, 2019 19:22
Copy link
Contributor

@ylwu-amzn ylwu-amzn left a comment

Choose a reason for hiding this comment

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

LGTM

@@ -180,9 +180,12 @@ export default class Monitors extends Component {

updateMonitor(item, update) {
const { httpClient } = this.props;
const { id, version, monitor } = item;
Copy link
Contributor

Choose a reason for hiding this comment

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

Let's call them seqNo and primaryTerm, the ifSeqNo/ifPrimaryTerm is just for the API query term. The actual values represent seqNo and primaryTerm.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Sure. Will fix it part of next PR. This is how currently it is being used rest of the place. Will change it together.

@mihirsoni mihirsoni merged commit a8daf0a into opendistro-for-elasticsearch:master Jun 26, 2019
mihirsoni added a commit that referenced this pull request Jun 26, 2019
Pass seqNo and PrimaryTerm for updateMonitor API.
mihirsoni added a commit that referenced this pull request Jun 26, 2019
Pass seqNo and PrimaryTerm for updateMonitor API.
@mihirsoni mihirsoni deleted the fix-monitor-list branch June 27, 2019 15:40
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants