You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@iceberg.apache.org by GitBox <gi...@apache.org> on 2022/02/23 18:39:33 UTC
[GitHub] [iceberg] kbendick opened a new pull request #4210: Infra - Trigger CI using on pull_request instead of on push
kbendick opened a new pull request #4210:
URL: https://github.com/apache/iceberg/pull/4210
Right now, we use both `push` and `pull_request` event to trigger our github actions.
This causes us to duplicate our `paths-ignore` in every file.
Additionally, as we start to use more github action features, such as in https://github.com/apache/iceberg/pull/4207, it would be good to reduce the number of events that trigger workflows so that the `github` context has consistent fields. For example, `github.head_ref` is only defined for `pull_request` and not `push` or any other trigger.
Additionally, using `push` causes CI to run in forks etc when people push to their branch.
Using the `pull_request` type of `synchronize` will make it so that CI is triggered when people update their PR, which was the original intention behind using `push` event.
cc @snazy @nastra @rdblue
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
[GitHub] [iceberg] kbendick commented on a change in pull request #4210: Infra - Trigger CI using on pull_request instead of on push
Posted by GitBox <gi...@apache.org>.
kbendick commented on a change in pull request #4210:
URL: https://github.com/apache/iceberg/pull/4210#discussion_r814302875
##########
File path: .github/workflows/flink-ci.yml
##########
@@ -19,27 +19,13 @@
name: "Flink CI"
on:
- push:
Review comment:
I was going to make a separate PR that would run the full test suite on master (so merges to master don't get preempted), but for now that's a better idea.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
[GitHub] [iceberg] nastra commented on pull request #4210: Infra - Trigger CI using on pull_request, saving push for master branch and tags (releases)
Posted by GitBox <gi...@apache.org>.
nastra commented on pull request #4210:
URL: https://github.com/apache/iceberg/pull/4210#issuecomment-1053952819
I'm totally fine handling this in a follow-up, just wanted to raise this
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
[GitHub] [iceberg] snazy commented on pull request #4210: Infra - Trigger CI using on pull_request, saving push for master branch and tags (releases)
Posted by GitBox <gi...@apache.org>.
snazy commented on pull request #4210:
URL: https://github.com/apache/iceberg/pull/4210#issuecomment-1057203408
> My only concern with this is that I think presently, tests get run in your fork any time you push to any branch. Do we think it's possible to do `apache/**` or something?
Ah, right. Forks would run CI on every branch then as well. (Why doesn't GH disable actions on forks automatically (anymore)...).
Fair enough, let's stick to `master` + tags for now and consider using a common prefix for release-branches w/ CI later / in a follow-up.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
[GitHub] [iceberg] kbendick commented on pull request #4210: Infra - Trigger CI using on pull_request instead of on push
Posted by GitBox <gi...@apache.org>.
kbendick commented on pull request #4210:
URL: https://github.com/apache/iceberg/pull/4210#issuecomment-1050311870
This, combined with https://github.com/apache/iceberg/pull/4207, will ensure that _all_ CI suites are run to completion for every merge to master cc @rdblue
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
[GitHub] [iceberg] kbendick commented on pull request #4210: Infra - Trigger CI using on pull_request, saving push for master branch and tags (releases)
Posted by GitBox <gi...@apache.org>.
kbendick commented on pull request #4210:
URL: https://github.com/apache/iceberg/pull/4210#issuecomment-1053937926
> @kbendick do we want to run CI on pushes to branches like https://github.com/apache/iceberg/tree/0.12.x or https://github.com/apache/iceberg/tree/0.13.x? If the answer is no, then I think we can go ahead and merge this.
Hmmmm.... In the release workflow, there should be a corresponding tag for each of those branches. We don't presently commit to those branches outside of a release that I can think of, which would be covered by a tag.
That said, I think it's not a bad idea to consider running CI for release-like branches, of the form `0.X.Y` or `X.Y.Z`:
1) The tests will be accessible in more places / more consistent locations.
2) People who run forks might apply patches to the OSS 0.13.x branch (applying additional patches on a release). If they presently use the Github Actions defined here, I would consider that a valid use case.
3) Failing CI workflows have more chance to be caught in places that matter.
So I am fine with merging this now, and then opening an issue to consider also adding CI triggers when we push to a release branch, of the form `0.Y.Z`. As we don't presently use feature branches / separate active release branches (outside of during the actual release process, which will be picked up by the tag event) , it's not needed outside of those additional benefits as far as I can think of.
Possibly we can just cover every branch that is within the `apache` repo, though that wouldn't handle the forks.
I'm ok with opening an issue to resolve that question in a follow up, though if you'd like to consider it now (e.g. if this patch would break your CI workflow for forked changes to release branches for example), then I'm open to updating it here. Otherwise I think it's fine to wait as we won't have a release in the next week. Do you think it's necessary to handle release-like branches now or can we open an issue and get to it next week or so @nastra? 🙂
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
[GitHub] [iceberg] rdblue commented on pull request #4210: Infra - Trigger CI using on pull_request, saving push for master branch and tags (releases)
Posted by GitBox <gi...@apache.org>.
rdblue commented on pull request #4210:
URL: https://github.com/apache/iceberg/pull/4210#issuecomment-1061297512
Thanks! Is it possible to pattern match on branches like this does for tags? All of the release candidates start with `apache-iceberg-*`
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
[GitHub] [iceberg] kbendick commented on pull request #4210: Infra - Trigger CI using on pull_request, saving push for master branch and tags (releases)
Posted by GitBox <gi...@apache.org>.
kbendick commented on pull request #4210:
URL: https://github.com/apache/iceberg/pull/4210#issuecomment-1061320792
> Thanks! Is it possible to pattern match on branches like this does for tags? All of the release candidates start with `apache-iceberg-*`
Yes it is. I can open a PR now. đź‘Ť
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
[GitHub] [iceberg] kbendick commented on pull request #4210: Infra - Trigger CI using on pull_request, saving push for master branch and tags (releases)
Posted by GitBox <gi...@apache.org>.
kbendick commented on pull request #4210:
URL: https://github.com/apache/iceberg/pull/4210#issuecomment-1061328646
> > Thanks! Is it possible to pattern match on branches like this does for tags? All of the release candidates start with `apache-iceberg-*`
>
> Yes it is. I can open a PR now. đź‘Ť
Opened a PR for release branches: https://github.com/apache/iceberg/pull/4287
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
[GitHub] [iceberg] kbendick commented on a change in pull request #4210: Infra - Trigger CI using on pull_request instead of on push
Posted by GitBox <gi...@apache.org>.
kbendick commented on a change in pull request #4210:
URL: https://github.com/apache/iceberg/pull/4210#discussion_r814304434
##########
File path: .github/workflows/flink-ci.yml
##########
@@ -19,27 +19,13 @@
name: "Flink CI"
on:
- push:
Review comment:
I'm going to remove the `paths-ignore`, as when I spoke to @rdblue about this a while ago, the ideal situation was to run all tests after merging to master.
That way, we don't have to keep both lists synchronized. Then eventually we can have a separate PR workflow just for merging to master and for tagging (new releases).
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
[GitHub] [iceberg] kbendick commented on pull request #4210: Infra - Trigger CI using on pull_request, saving push for master branch only
Posted by GitBox <gi...@apache.org>.
kbendick commented on pull request #4210:
URL: https://github.com/apache/iceberg/pull/4210#issuecomment-1050570508
> changes LGTM, but how do we want to deal with running CI on release branches, such as `0.14.x`? Once this change is in, merges to those branches won't trigger separate CI runs and we would then rely on the PR CI runs. If that's desired then we can go ahead and merge, but I just wanted to point out the implication of this change here.
That’s a good point. We _do_ make a commit of the version.txt file as part of each release cycle, which I would say is the canonical “commit” of the release (it is the SHA released I believe).
That said, GitHub Actions have gotten _alot_ better and so we can also have CI run on `tag` and then have everything run.
**TLDR** - Let’s open a follow up issue to create a single CI suite that runs everything in CI / tests when we merge into master / on tag (really, regardless of the change set, we should just run all of CI or at least all parts if not all versions when we merge). We can limit the time it takes by changing the run to something simpler (I think we duplicate some build and test runs when we run `Java` and some of the other CI’s together if they share code).
Or we can go all out and make a `controller` ci and convert everything else to run on workflow run. I started an attempt at it, but at the time GitHub Actions were somewhat more beta and I worried the knowledge wouldn’t be there. But we could save a good amount of CI time this way. We’d save CI time, as currently a failure in `Spark` will kill the other parallel Spark jobs (from the same matrix), but not `Flink` or other CI jobs.
Java CI fails within minutes and is the most common failure (checkstyle, core, etc). So we could save _alot_ of unnecessary runs this way. And now we at least are already using on `workflow_call` so the syntax isn’t as unknown at a group level like when I did it before.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
[GitHub] [iceberg] kbendick commented on a change in pull request #4210: Infra - Trigger CI using on pull_request, saving push for master branch and tags (releases)
Posted by GitBox <gi...@apache.org>.
kbendick commented on a change in pull request #4210:
URL: https://github.com/apache/iceberg/pull/4210#discussion_r815032777
##########
File path: .github/workflows/flink-ci.yml
##########
@@ -20,25 +20,8 @@
name: "Flink CI"
on:
push:
- paths-ignore:
- - '.github/workflows/python-ci.yml'
- - '.github/workflows/spark-ci.yml'
- - '.github/workflows/hive-ci.yml'
- - '.github/workflows/cancel-duplicate-workflow-runs.yml'
- - '.gitignore'
- - 'dev/**'
- - 'mr/**'
- - 'hive3/**'
- - 'hive3-orc-bundle/**'
- - 'hive-runtime/**'
- - 'spark/**'
- - 'pig/**'
- - 'python/**'
- - 'python_legacy/**'
- - 'docs/**'
- - '.gitattributes'
- - 'README.md'
- - 'CONTRIBUTING.md'
+ branches:
+ - 'master'
Review comment:
For now, I just added `'**'` to catch all, as that's the only thing we use tags for presently anyway.
The tags from the examples use two asterisks, so I went with that: https://docs.github.com/en/actions/using-workflows/events-that-trigger-workflows#running-your-workflow-only-when-a-push-of-specific-tags-occurs
I can try merging this into my own fork and then pushing a tag to ensure it works or we can just merge it and then check in the fork (less work). I'm open either way.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
[GitHub] [iceberg] snazy commented on pull request #4210: Infra - Trigger CI using on pull_request, saving push for master branch and tags (releases)
Posted by GitBox <gi...@apache.org>.
snazy commented on pull request #4210:
URL: https://github.com/apache/iceberg/pull/4210#issuecomment-1054007674
> That said, I think it's not a bad idea to consider running CI for release-like branches, of the form `0.X.Y` or `X.Y.Z`:
Looks like it would just require us to replace `- `master'` with `- '**'` in the .yml files, since there are no feature branches or so. Maybe add that to this PR and we're good on "CI on all branches" front :)
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
[GitHub] [iceberg] nastra commented on a change in pull request #4210: Infra - Trigger CI using on pull_request instead of on push
Posted by GitBox <gi...@apache.org>.
nastra commented on a change in pull request #4210:
URL: https://github.com/apache/iceberg/pull/4210#discussion_r813667318
##########
File path: .github/workflows/flink-ci.yml
##########
@@ -19,27 +19,13 @@
name: "Flink CI"
on:
- push:
- paths-ignore:
- - '.github/workflows/python-ci.yml'
- - '.github/workflows/spark-ci.yml'
- - '.github/workflows/hive-ci.yml'
- - '.github/workflows/cancel-duplicate-workflow-runs.yml'
- - '.gitignore'
- - 'dev/**'
- - 'mr/**'
- - 'hive3/**'
- - 'hive3-orc-bundle/**'
- - 'hive-runtime/**'
- - 'spark/**'
- - 'pig/**'
- - 'python/**'
- - 'python_legacy/**'
- - 'docs/**'
- - '.gitattributes'
- - 'README.md'
- - 'CONTRIBUTING.md'
pull_request:
+ types:
+ - opened
+ - reopened
+ - ready_for_review
Review comment:
in that case we could probably omit all of the types, since `opened` / `reopened` / `synchronize` are the defaults for `pull_request` according to https://docs.github.com/en/actions/using-workflows/events-that-trigger-workflows
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
[GitHub] [iceberg] kbendick commented on pull request #4210: Infra - Trigger CI using on pull_request, saving push for master branch and tags (releases)
Posted by GitBox <gi...@apache.org>.
kbendick commented on pull request #4210:
URL: https://github.com/apache/iceberg/pull/4210#issuecomment-1055824590
> > That said, I think it's not a bad idea to consider running CI for release-like branches, of the form `0.X.Y` or `X.Y.Z`:
>
> Looks like it would just require us to replace `- 'master'` with `- '**'` in the .yml files, since there are no feature branches or so. Maybe add that to this PR and we're good on "CI on all branches" front :)
My only concern with this is that I think presently, tests get run in your fork any time you push to any branch. Do we think it's possible to do `apache/**` or something?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
[GitHub] [iceberg] kbendick commented on a change in pull request #4210: Infra - Trigger CI using on pull_request instead of on push
Posted by GitBox <gi...@apache.org>.
kbendick commented on a change in pull request #4210:
URL: https://github.com/apache/iceberg/pull/4210#discussion_r814302155
##########
File path: .github/workflows/flink-ci.yml
##########
@@ -19,27 +19,13 @@
name: "Flink CI"
on:
- push:
- paths-ignore:
- - '.github/workflows/python-ci.yml'
- - '.github/workflows/spark-ci.yml'
- - '.github/workflows/hive-ci.yml'
- - '.github/workflows/cancel-duplicate-workflow-runs.yml'
- - '.gitignore'
- - 'dev/**'
- - 'mr/**'
- - 'hive3/**'
- - 'hive3-orc-bundle/**'
- - 'hive-runtime/**'
- - 'spark/**'
- - 'pig/**'
- - 'python/**'
- - 'python_legacy/**'
- - 'docs/**'
- - '.gitattributes'
- - 'README.md'
- - 'CONTRIBUTING.md'
pull_request:
+ types:
+ - opened
+ - reopened
+ - ready_for_review
Review comment:
Oh I thought that `synchronize` wasn't a default. Then yeah, we can omit them. Good catch!
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
[GitHub] [iceberg] kbendick commented on a change in pull request #4210: Infra - Trigger CI using on pull_request, saving push for master branch and tags (releases)
Posted by GitBox <gi...@apache.org>.
kbendick commented on a change in pull request #4210:
URL: https://github.com/apache/iceberg/pull/4210#discussion_r815030774
##########
File path: .github/workflows/flink-ci.yml
##########
@@ -19,27 +19,13 @@
name: "Flink CI"
on:
- push:
Review comment:
Ok. I added the following which should cover any tags.
```yaml
on:
push:
branches:
- 'master'
tags:
- '**'
```
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
[GitHub] [iceberg] kbendick commented on a change in pull request #4210: Infra - Trigger CI using on pull_request instead of on push
Posted by GitBox <gi...@apache.org>.
kbendick commented on a change in pull request #4210:
URL: https://github.com/apache/iceberg/pull/4210#discussion_r814304434
##########
File path: .github/workflows/flink-ci.yml
##########
@@ -19,27 +19,13 @@
name: "Flink CI"
on:
- push:
Review comment:
I'm going to remove the `paths-ignore`, as when I spoke to @rdblue about this a while ago, the ideal situation was to run all tests after merging to master.
That way, we don't have to keep both lists synchronized. We can maybe still leave them for python or something for example (though that one is pretty fast).
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
[GitHub] [iceberg] snazy commented on a change in pull request #4210: Infra - Trigger CI using on pull_request, saving push for master branch only
Posted by GitBox <gi...@apache.org>.
snazy commented on a change in pull request #4210:
URL: https://github.com/apache/iceberg/pull/4210#discussion_r814642358
##########
File path: .github/workflows/flink-ci.yml
##########
@@ -20,25 +20,8 @@
name: "Flink CI"
on:
push:
- paths-ignore:
- - '.github/workflows/python-ci.yml'
- - '.github/workflows/spark-ci.yml'
- - '.github/workflows/hive-ci.yml'
- - '.github/workflows/cancel-duplicate-workflow-runs.yml'
- - '.gitignore'
- - 'dev/**'
- - 'mr/**'
- - 'hive3/**'
- - 'hive3-orc-bundle/**'
- - 'hive-runtime/**'
- - 'spark/**'
- - 'pig/**'
- - 'python/**'
- - 'python_legacy/**'
- - 'docs/**'
- - '.gitattributes'
- - 'README.md'
- - 'CONTRIBUTING.md'
+ branches:
+ - 'master'
Review comment:
I suspect that adding
```
- '*.x'
```
could work to have CI run on release branches. If it might make sense to move to a different naming pattern for release branches (like `release-x.y.z`).
##########
File path: .github/workflows/flink-ci.yml
##########
@@ -20,25 +20,8 @@
name: "Flink CI"
on:
push:
- paths-ignore:
- - '.github/workflows/python-ci.yml'
- - '.github/workflows/spark-ci.yml'
- - '.github/workflows/hive-ci.yml'
- - '.github/workflows/cancel-duplicate-workflow-runs.yml'
- - '.gitignore'
- - 'dev/**'
- - 'mr/**'
- - 'hive3/**'
- - 'hive3-orc-bundle/**'
- - 'hive-runtime/**'
- - 'spark/**'
- - 'pig/**'
- - 'python/**'
- - 'python_legacy/**'
- - 'docs/**'
- - '.gitattributes'
- - 'README.md'
- - 'CONTRIBUTING.md'
+ branches:
Review comment:
Adding this should work for tags, but I that's not necessary when CI's run on release branches as well and the tag is on a release branch.
```
tags:
- 'apache-iceberg-*'
```
##########
File path: .github/workflows/flink-ci.yml
##########
@@ -19,27 +19,13 @@
name: "Flink CI"
on:
- push:
Review comment:
For master/release-branch builds that makes sense to me.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
[GitHub] [iceberg] snazy commented on a change in pull request #4210: Infra - Trigger CI using on pull_request instead of on push
Posted by GitBox <gi...@apache.org>.
snazy commented on a change in pull request #4210:
URL: https://github.com/apache/iceberg/pull/4210#discussion_r813575200
##########
File path: .github/workflows/flink-ci.yml
##########
@@ -19,27 +19,13 @@
name: "Flink CI"
on:
- push:
Review comment:
This would stop running CI on the master branch. Just needs another attribute:
```
branches: [ master ]
```
##########
File path: .github/workflows/flink-ci.yml
##########
@@ -19,27 +19,13 @@
name: "Flink CI"
on:
- push:
- paths-ignore:
- - '.github/workflows/python-ci.yml'
- - '.github/workflows/spark-ci.yml'
- - '.github/workflows/hive-ci.yml'
- - '.github/workflows/cancel-duplicate-workflow-runs.yml'
- - '.gitignore'
- - 'dev/**'
- - 'mr/**'
- - 'hive3/**'
- - 'hive3-orc-bundle/**'
- - 'hive-runtime/**'
- - 'spark/**'
- - 'pig/**'
- - 'python/**'
- - 'python_legacy/**'
- - 'docs/**'
- - '.gitattributes'
- - 'README.md'
- - 'CONTRIBUTING.md'
pull_request:
+ types:
+ - opened
+ - reopened
+ - ready_for_review
Review comment:
I suspect, it's fine to omit `ready_for_review`, because it's only a change on "PR metadata" and CI should have already been triggered.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
[GitHub] [iceberg] kbendick commented on a change in pull request #4210: Infra - Trigger CI using on pull_request, saving push for master branch and tags (releases)
Posted by GitBox <gi...@apache.org>.
kbendick commented on a change in pull request #4210:
URL: https://github.com/apache/iceberg/pull/4210#discussion_r815031463
##########
File path: .github/workflows/flink-ci.yml
##########
@@ -20,25 +20,8 @@
name: "Flink CI"
on:
push:
- paths-ignore:
- - '.github/workflows/python-ci.yml'
- - '.github/workflows/spark-ci.yml'
- - '.github/workflows/hive-ci.yml'
- - '.github/workflows/cancel-duplicate-workflow-runs.yml'
- - '.gitignore'
- - 'dev/**'
- - 'mr/**'
- - 'hive3/**'
- - 'hive3-orc-bundle/**'
- - 'hive-runtime/**'
- - 'spark/**'
- - 'pig/**'
- - 'python/**'
- - 'python_legacy/**'
- - 'docs/**'
- - '.gitattributes'
- - 'README.md'
- - 'CONTRIBUTING.md'
+ branches:
Review comment:
Oh good catch. I just added a tag of `**`. I can update it to the tag you suggested if you'd like.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
[GitHub] [iceberg] kbendick commented on pull request #4210: Infra - Trigger CI using on pull_request, saving push for master branch and tags (releases)
Posted by GitBox <gi...@apache.org>.
kbendick commented on pull request #4210:
URL: https://github.com/apache/iceberg/pull/4210#issuecomment-1058401733
> > My only concern with this is that I think presently, tests get run in your fork any time you push to any branch. Do we think it's possible to do `apache/**` or something?
>
> Ah, right. Forks would run CI on every branch then as well. (Why doesn't GH disable actions on forks automatically (anymore)...). Fair enough, let's stick to `master` + tags for now and consider using a common prefix for release-branches w/ CI later / in a follow-up.
That works for me. Realistically, the tags thing shouldn't come up until the next release cycle anyway.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
[GitHub] [iceberg] kbendick commented on pull request #4210: Infra - Trigger CI using on pull_request instead of on push
Posted by GitBox <gi...@apache.org>.
kbendick commented on pull request #4210:
URL: https://github.com/apache/iceberg/pull/4210#issuecomment-1049103854
The one thing I think this might affect is that we currently implicitly rely on the `push` event for running CI when merging into master.
This has the unfortunate side effect of cancelling merges into master if another merge happens (because of the cancel duplicates work). Ideally, all CI would run to completion after merging into master, so commits don't look like they failed CI.
I will also open a PR to fix that as well.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
[GitHub] [iceberg] nastra commented on pull request #4210: Infra - Trigger CI using on pull_request, saving push for master branch and tags (releases)
Posted by GitBox <gi...@apache.org>.
nastra commented on pull request #4210:
URL: https://github.com/apache/iceberg/pull/4210#issuecomment-1053922116
@kbendick do we want to run CI on pushes to branches like https://github.com/apache/iceberg/tree/0.12.x or https://github.com/apache/iceberg/tree/0.13.x? If the answer is no, then I think we can go ahead and merge this.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
[GitHub] [iceberg] snazy edited a comment on pull request #4210: Infra - Trigger CI using on pull_request, saving push for master branch and tags (releases)
Posted by GitBox <gi...@apache.org>.
snazy edited a comment on pull request #4210:
URL: https://github.com/apache/iceberg/pull/4210#issuecomment-1054007674
> That said, I think it's not a bad idea to consider running CI for release-like branches, of the form `0.X.Y` or `X.Y.Z`:
Looks like it would just require us to replace `- 'master'` with `- '**'` in the .yml files, since there are no feature branches or so. Maybe add that to this PR and we're good on "CI on all branches" front :)
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
[GitHub] [iceberg] kbendick commented on a change in pull request #4210: Infra - Trigger CI using on pull_request instead of on push
Posted by GitBox <gi...@apache.org>.
kbendick commented on a change in pull request #4210:
URL: https://github.com/apache/iceberg/pull/4210#discussion_r814302875
##########
File path: .github/workflows/flink-ci.yml
##########
@@ -19,27 +19,13 @@
name: "Flink CI"
on:
- push:
Review comment:
I was going to make a separate PR that would run the full test suite on master (so merges to master don't get preempted), but for now that's a better idea (and works well with the updates from https://github.com/apache/iceberg/pull/4207).
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
[GitHub] [iceberg] kbendick commented on pull request #4210: Infra - Trigger CI using on pull_request, saving push for master branch only
Posted by GitBox <gi...@apache.org>.
kbendick commented on pull request #4210:
URL: https://github.com/apache/iceberg/pull/4210#issuecomment-1050572830
Here is the PR where I was working on getting all of CI to fail early if one task in one CI group fails: https://github.com/apache/iceberg/pull/3524 @nastra
I can’t think of a better way to accomplish that (failing early when one task fails) than workflow call. We can repurpose that PR if we’d like, especially now that GitHub actions are a bit simpler / require less specification. Let me know what you think, as realistically we spend a decent amount of time in these files and previously I was worried that too few people would know the syntax. But now we already use it in the workflow you authored I believe. This would just be a very extreme version of that haha.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org
[GitHub] [iceberg] rdblue merged pull request #4210: Infra - Trigger CI using on pull_request, saving push for master branch and tags (releases)
Posted by GitBox <gi...@apache.org>.
rdblue merged pull request #4210:
URL: https://github.com/apache/iceberg/pull/4210
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org