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.

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

Replaced if_seq_num to if_seq_no #166

Merged
merged 2 commits into from
Nov 4, 2022

Conversation

noname4life
Copy link
Contributor

Signed-off by: Kyle Darryl Aguilar aguilarkyledarryl@gmail.com

Description

Renamed the sequence number struct tag to resolve issues when performing optimistic concurrency control

Issues Resolved

Resolves #165

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.
For more information on following Developer Certificate of Origin and signing off your commits, please check here.

@noname4life noname4life requested review from a team and svencowart as code owners September 30, 2022 19:23
dblock
dblock previously approved these changes Oct 3, 2022
@dblock
Copy link
Member

dblock commented Oct 3, 2022

Please amend the commit with -s (DCO).

Is this something we can/should have a unit test for?

@noname4life
Copy link
Contributor Author

Thank you for the review and noted on this. I have amended the commit and have added a unit test for this change.

@noname4life noname4life requested review from dblock and removed request for svencowart October 5, 2022 14:49
@dblock
Copy link
Member

dblock commented Oct 5, 2022

Something's off with CI/CD 🤔

Looks like DCO is still failing though, maybe squash your commits locally and force push? LMK if you need help?

@noname4life
Copy link
Contributor Author

It seems to have been caused by certain configurations in my local. Updated the commit to address this.

@dblock
Copy link
Member

dblock commented Oct 20, 2022

The linter is failing now, take a look?

@noname4life
Copy link
Contributor Author

noname4life commented Oct 23, 2022

Hi @dblock , the failing linter seems to be caused by the go version 1.16.2 in the lint.yml file in reference to the returned error of the lint github action which indicated a required go version of 1.17.

I have pushed a commit to bump the go version.

@VachaShah
Copy link
Collaborator

Hi @noname4life, can you please rebase to get the latest changes from main? Thank you!

@noname4life
Copy link
Contributor Author

Hi @VachaShah, I have rebased this branch with the updates from main. Kindly let me know if I have missed anything. Thank you!

Copy link
Member

@dblock dblock left a comment

Choose a reason for hiding this comment

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

You'll need an entry in CHANGELOG, please (this is new)

Signed-off-by: Kyle Darryl Aguilar <aguilarkyledarryl@gmail.com>
Signed-off-by: Kyle Darryl Aguilar <aguilarkyledarryl@gmail.com>
@noname4life
Copy link
Contributor Author

Hi @dblock @VachaShah , I have added an entry in CHANGELOG

@VachaShah
Copy link
Collaborator

Hi @dblock @VachaShah , I have added an entry in CHANGELOG

Thank you @noname4life, re-running the workflows

@VachaShah VachaShah requested a review from VijayanB November 4, 2022 19:23
@VachaShah VachaShah merged commit 2951240 into opensearch-project:main Nov 4, 2022
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.

[BUG] Unknown Parameter [if_seq_num] Error When Using Optimistic Concurrency Control
4 participants