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

HDDS-10482. OMRequestTestUtils.createOmKeyInfo should set key modification time #6343

Merged
merged 2 commits into from
Mar 7, 2024

Conversation

smengcl
Copy link
Contributor

@smengcl smengcl commented Mar 6, 2024

What changes were proposed in this pull request?

Set mod time in OMRequestTestUtils.createOmKeyInfo to fix the regression in test util that caused test failure in a feature branch. See jira description for more backgrounds.

What is the link to the Apache JIRA

https://issues.apache.org/jira/browse/HDDS-10482

How was this patch tested?

  • Added assertion to prevent such regression from happening on the master branch again.

@smengcl smengcl added the test label Mar 6, 2024
@smengcl smengcl requested a review from jojochuang March 6, 2024 23:29
@jojochuang
Copy link
Contributor

LGTM. Build is broken by a recent commit. Let's trigger a rebuild after the broken commit is reverted/fixed.

@smengcl
Copy link
Contributor Author

smengcl commented Mar 7, 2024

I've rebased this PR to the master branch before the breaking commit. In order to run CI independent of the breakage.

Copy link
Contributor

@adoroszlai adoroszlai left a comment

Choose a reason for hiding this comment

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

Thanks @smengcl for spotting this.

I've rebased this PR to the master branch before the breaking commit. In order to run CI independent of the breakage.

That doesn't work, PRs test patch on top of current master. Build error is now fixed on master.

@adoroszlai adoroszlai changed the title HDDS-10482. HDDS-10226 should set mod time for OMRequestTestUtils.createOmKeyInfo HDDS-10482. OMRequestTestUtils.createOmKeyInfo should set key modification time Mar 7, 2024
@smengcl
Copy link
Contributor Author

smengcl commented Mar 7, 2024

Thanks @smengcl for spotting this.

I've rebased this PR to the master branch before the breaking commit. In order to run CI independent of the breakage.

That doesn't work, PRs test patch on top of current master. Build error is now fixed on master.

Interesting. The trick works on my fork's branch here:

https://github.com/smengcl/hadoop-ozone/actions/runs/8181320975

I guess the PR CI just stick to the master branch when it is posted?

Anyway will rebase again.

@adoroszlai
Copy link
Contributor

Interesting. The trick works on my fork's branch

Build for push (usually happens in forks, or when PR is merged) uses exactly the commit that is pushed. Build for PR works as I described.

@adoroszlai adoroszlai merged commit 7c8160f into apache:master Mar 7, 2024
27 checks passed
@adoroszlai
Copy link
Contributor

Thanks @smengcl for the fix.

@smengcl
Copy link
Contributor Author

smengcl commented Mar 7, 2024

Thanks @adoroszlai for the review and comments!

@smengcl
Copy link
Contributor Author

smengcl commented Mar 7, 2024

Thanks @jojochuang for the review as well!

jojochuang pushed a commit to jojochuang/ozone that referenced this pull request Mar 15, 2024
…ation time (apache#6343)

(cherry picked from commit 7c8160f)
Change-Id: I40d15e964681a120856c4d5a45e87f811bedcbbb
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants