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

Fix the hoodie ci s3 test failed. #446

Open
wants to merge 5 commits into
base: master
Choose a base branch
from
Open

Fix the hoodie ci s3 test failed. #446

wants to merge 5 commits into from

Conversation

horizonzy
Copy link
Member

Motivation

The HoodieWriterTest will run ci with s3, but the env is not set, so the test can't pass. I think we can disable the s3 in the ci.

Documentation

Check the box below.

Need to update docs?

  • doc-required

    (If you need help on updating docs, create a doc issue)

  • no-need-doc

    (Please explain why)

  • doc

    (If this PR contains doc changes)

@hangc0276
Copy link
Collaborator

The CI can pass when one branch in this project wants to merge to the master, but it will fail when the forked repo wants to merge PR to the master. It is not friendly for community users to contribute PR to this repo. I think we can disable the S3 test. Do you have any ideas? @zymap

@zymap
Copy link
Member

zymap commented Nov 2, 2023

We can run that test when the PR is merged into the master, and for pull-request, we can skip it.

We can separate the cloud-related test, and only trigger it running using the condition:

    - name: cloud test after build
      if: github.event_name == 'push'
      env:
        CLOUD_BUCKET_NAME: ${{ secrets.CLOUD_BUCKET_NAME }}
        AWS_SECRET_KEY: ${{ secrets.AWS_SECRET_KEY }}
        AWS_ACCESS_KEY_ID: ${{ secrets.AWS_ACCESS_KEY_ID }}
      run: mvn test -Pcloud -Dtest='TestS3Cloud.java'

Then, run the remaining tests in the usual pull request checks.

@hangc0276
Copy link
Collaborator

hangc0276 commented Nov 2, 2023

We can run that test when the PR is merged into the master, and for pull-request, we can skip it.

We can separate the cloud-related test, and only trigger it running using the condition:

    - name: cloud test after build
      if: github.event_name == 'push'
      env:
        CLOUD_BUCKET_NAME: ${{ secrets.CLOUD_BUCKET_NAME }}
        AWS_SECRET_KEY: ${{ secrets.AWS_SECRET_KEY }}
        AWS_ACCESS_KEY_ID: ${{ secrets.AWS_ACCESS_KEY_ID }}
      run: mvn test -Pcloud -Dtest='TestS3Cloud.java'

Then, run the remaining tests in the usual pull request checks.

make sense
@horizonzy Would you please update this PR? thanks.

@horizonzy
Copy link
Member Author

I'm not sure I got the idea clearly, please help to review it. Thanks @zymap

hangc0276
hangc0276 previously approved these changes Nov 3, 2023
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.

None yet

3 participants