You are viewing a plain text version of this content. The canonical link for it is here.
Posted to github@arrow.apache.org by GitBox <gi...@apache.org> on 2022/08/25 15:47:18 UTC

[GitHub] [arrow] kevingurney opened a new pull request, #13975: WIP: ARROW-15693: [Dev] Update crossbow templates to use master or main

kevingurney opened a new pull request, #13975:
URL: https://github.com/apache/arrow/pull/13975

   # Note
   
   1. **This pull request is a work in progress. Please do not merge yet.**
   
   # Overview
   
   This pull request:
   
   1. Removes hard-coded dependencies on "master" as the default branch name in the crossbow infrastructure and CI template files.
   
   # Implementation
   
   1. Removed comment/text references to "master" branch, including URLs to other repositories.
   2. Modified `core.py` to add `default_branch` property and a method `is_default_branch`, for checking whether on the default branch named, to the `Target` class.
   3. Modified CI template files to use the new `is_default_branch` function to check whether on the default branch.
   
   # Testing
   
   1. Using `lafiona/crossbow` as a queue repository for qualification.
   2. Running modified template jobs.
   
   # Future Directions
   
   1. Remove "master" from `default_branch` name property of `Target` class.
   2. Remove all remaining uses of "master" terminology in crossbow.
   
   # Notes
   
   1. Thank you to @lafiona for her help with this pull request!
   2. Due to unexpected technical issues, we opened this pull request as a follow up to https://github.com/apache/arrow/pull/13750. Please see https://github.com/apache/arrow/pull/13750 for more discussion regarding qualification efforts.


-- 
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: github-unsubscribe@arrow.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [arrow] amol- merged pull request #13975: ARROW-15693: [Dev] Update crossbow templates to use master or main

Posted by GitBox <gi...@apache.org>.
amol- merged PR #13975:
URL: https://github.com/apache/arrow/pull/13975


-- 
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: github-unsubscribe@arrow.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [arrow] kevingurney commented on pull request #13975: ARROW-15693: [Dev] Update crossbow templates to use master or main

Posted by GitBox <gi...@apache.org>.
kevingurney commented on PR #13975:
URL: https://github.com/apache/arrow/pull/13975#issuecomment-1232044063

   I'm very sorry @raulcd - I am not sure why I didn't notice the linting issues before! Thanks for pointing that out! I'll fix them as soon as possible.


-- 
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: github-unsubscribe@arrow.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [arrow] kevingurney commented on pull request #13975: ARROW-15693: [Dev] Update crossbow templates to use master or main

Posted by GitBox <gi...@apache.org>.
kevingurney commented on PR #13975:
URL: https://github.com/apache/arrow/pull/13975#issuecomment-1238319188

   Thanks for re-running the benchmarks @raulcd! I was just about to re-run them myself, but had to step away.


-- 
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: github-unsubscribe@arrow.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [arrow] kevingurney commented on a diff in pull request #13975: ARROW-15693: [Dev] Update crossbow templates to use master or main

Posted by GitBox <gi...@apache.org>.
kevingurney commented on code in PR #13975:
URL: https://github.com/apache/arrow/pull/13975#discussion_r959531022


##########
dev/tasks/conda-recipes/azure.clean.yml:
##########
@@ -1,3 +1,5 @@
+{% import 'macros.jinja' as macros with context %}

Review Comment:
   1. You are absolutely right! Importing the macros are not required anymore. This was an oversight on my part - I left this in by accident from a previous version of the code which used a jinja macro instead of a method on the `Target` object to check whether the current branch is the default branch. Since leaving these import statements in didn't cause any obvious issues, I completely forgot to remove them. My apologies! Thank you for catching this!
   
   2. Of course - I will add the License header.



-- 
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: github-unsubscribe@arrow.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [arrow] ursabot commented on pull request #13975: ARROW-15693: [Dev] Update crossbow templates to use master or main

Posted by GitBox <gi...@apache.org>.
ursabot commented on PR #13975:
URL: https://github.com/apache/arrow/pull/13975#issuecomment-1238171081

   ['Python', 'R'] benchmarks have high level of regressions.
   [ursa-i9-9960x](https://conbench.ursa.dev/compare/runs/0eb54274d803435ebf5ee3d26f81327f...7c1fdbe523dc40a590bb2319891cf747/)
   


-- 
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: github-unsubscribe@arrow.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [arrow] ursabot commented on pull request #13975: ARROW-15693: [Dev] Update crossbow templates to use master or main

Posted by GitBox <gi...@apache.org>.
ursabot commented on PR #13975:
URL: https://github.com/apache/arrow/pull/13975#issuecomment-1238269120

   Benchmark runs are scheduled for baseline = 7e7b8e1f0a2f02046e0d0d6a0b9ada6d64648b9c and contender = 4575b26964abced698b2565cd16ab517cb521ca3. Results will be available as each benchmark for each run completes.
   Conbench compare runs links:
   [Scheduled] [ec2-t3-xlarge-us-east-2](https://conbench.ursa.dev/compare/runs/d784bfd21b5041e985391400daf42a01...718bb21e3a5d42aead8edb17f1c6eb25/)
   [Scheduled] [test-mac-arm](https://conbench.ursa.dev/compare/runs/423a737ed5584499979999025a605642...e9a4d80832f044788eafae0c7ce2cf74/)
   [Scheduled] [ursa-i9-9960x](https://conbench.ursa.dev/compare/runs/b51831c1b58a4de7a872dc36d59d7a91...59bad53829b04855b36c7267b4ec3d0c/)
   [Scheduled] [ursa-thinkcentre-m75q](https://conbench.ursa.dev/compare/runs/9e4392db1bf349a38c78436b86cef3b0...723cc72d87434f40a3ccc9ac16442bf4/)
   Buildkite builds:
   [Scheduled] [`4575b269` ec2-t3-xlarge-us-east-2](https://buildkite.com/apache-arrow/arrow-bci-benchmark-on-ec2-t3-xlarge-us-east-2/builds/1423)
   [Scheduled] [`4575b269` test-mac-arm](https://buildkite.com/apache-arrow/arrow-bci-benchmark-on-test-mac-arm/builds/1441)
   [Scheduled] [`4575b269` ursa-i9-9960x](https://buildkite.com/apache-arrow/arrow-bci-benchmark-on-ursa-i9-9960x/builds/1425)
   [Scheduled] [`4575b269` ursa-thinkcentre-m75q](https://buildkite.com/apache-arrow/arrow-bci-benchmark-on-ursa-thinkcentre-m75q/builds/1438)
   [Finished] [`7e7b8e1f` ec2-t3-xlarge-us-east-2](https://buildkite.com/apache-arrow/arrow-bci-benchmark-on-ec2-t3-xlarge-us-east-2/builds/1372)
   [Finished] [`7e7b8e1f` test-mac-arm](https://buildkite.com/apache-arrow/arrow-bci-benchmark-on-test-mac-arm/builds/1388)
   [Failed] [`7e7b8e1f` ursa-i9-9960x](https://buildkite.com/apache-arrow/arrow-bci-benchmark-on-ursa-i9-9960x/builds/1370)
   [Finished] [`7e7b8e1f` ursa-thinkcentre-m75q](https://buildkite.com/apache-arrow/arrow-bci-benchmark-on-ursa-thinkcentre-m75q/builds/1387)
   Supported benchmarks:
   ec2-t3-xlarge-us-east-2: Supported benchmark langs: Python, R. Runs only benchmarks with cloud = True
   test-mac-arm: Supported benchmark langs: C++, Python, R
   ursa-i9-9960x: Supported benchmark langs: Python, R, JavaScript
   ursa-thinkcentre-m75q: Supported benchmark langs: C++, Java
   


-- 
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: github-unsubscribe@arrow.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [arrow] raulcd commented on a diff in pull request #13975: ARROW-15693: [Dev] Update crossbow templates to use master or main

Posted by GitBox <gi...@apache.org>.
raulcd commented on code in PR #13975:
URL: https://github.com/apache/arrow/pull/13975#discussion_r959385679


##########
dev/tasks/conda-recipes/azure.clean.yml:
##########
@@ -1,3 +1,5 @@
+{% import 'macros.jinja' as macros with context %}

Review Comment:
   Is importing the macros necessary? I thought that as we are not using the macros anywhere on this file it shouldn't be necessary.
   Not related to your specific PR but I've noticed the License header is missing on this file, could you also add it?



-- 
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: github-unsubscribe@arrow.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [arrow] ursabot commented on pull request #13975: ARROW-15693: [Dev] Update crossbow templates to use master or main

Posted by GitBox <gi...@apache.org>.
ursabot commented on PR #13975:
URL: https://github.com/apache/arrow/pull/13975#issuecomment-1238170675

   Benchmark runs are scheduled for baseline = 23c5ceea785a96cc01c0fa976db1ffc7ff3e839f and contender = 3e40cd3648a5b4f6ee7203a6c408f08c0abe4696. 3e40cd3648a5b4f6ee7203a6c408f08c0abe4696 is a master commit associated with this PR. Results will be available as each benchmark for each run completes.
   Conbench compare runs links:
   [Finished :arrow_down:0.0% :arrow_up:0.0%] [ec2-t3-xlarge-us-east-2](https://conbench.ursa.dev/compare/runs/577c35ecd6774e7bb9f23538c6b51575...bc9464dc1d154697af0ea60e4329f9c6/)
   [Failed] [test-mac-arm](https://conbench.ursa.dev/compare/runs/30603e0f425744ccba792c38b11499a2...772c847dc1014bf790f7c90b881d9e60/)
   [Failed :arrow_down:1.37% :arrow_up:0.0%] [ursa-i9-9960x](https://conbench.ursa.dev/compare/runs/0eb54274d803435ebf5ee3d26f81327f...7c1fdbe523dc40a590bb2319891cf747/)
   [Finished :arrow_down:0.53% :arrow_up:0.0%] [ursa-thinkcentre-m75q](https://conbench.ursa.dev/compare/runs/9b3e1e420b6b4b279aab157864771bbf...94771fab861249808ded22b28cdfb35c/)
   Buildkite builds:
   [Finished] [`3e40cd36` ec2-t3-xlarge-us-east-2](https://buildkite.com/apache-arrow/arrow-bci-benchmark-on-ec2-t3-xlarge-us-east-2/builds/1421)
   [Failed] [`3e40cd36` test-mac-arm](https://buildkite.com/apache-arrow/arrow-bci-benchmark-on-test-mac-arm/builds/1439)
   [Failed] [`3e40cd36` ursa-i9-9960x](https://buildkite.com/apache-arrow/arrow-bci-benchmark-on-ursa-i9-9960x/builds/1423)
   [Finished] [`3e40cd36` ursa-thinkcentre-m75q](https://buildkite.com/apache-arrow/arrow-bci-benchmark-on-ursa-thinkcentre-m75q/builds/1436)
   [Finished] [`23c5ceea` ec2-t3-xlarge-us-east-2](https://buildkite.com/apache-arrow/arrow-bci-benchmark-on-ec2-t3-xlarge-us-east-2/builds/1420)
   [Failed] [`23c5ceea` test-mac-arm](https://buildkite.com/apache-arrow/arrow-bci-benchmark-on-test-mac-arm/builds/1438)
   [Failed] [`23c5ceea` ursa-i9-9960x](https://buildkite.com/apache-arrow/arrow-bci-benchmark-on-ursa-i9-9960x/builds/1422)
   [Finished] [`23c5ceea` ursa-thinkcentre-m75q](https://buildkite.com/apache-arrow/arrow-bci-benchmark-on-ursa-thinkcentre-m75q/builds/1435)
   Supported benchmarks:
   ec2-t3-xlarge-us-east-2: Supported benchmark langs: Python, R. Runs only benchmarks with cloud = True
   test-mac-arm: Supported benchmark langs: C++, Python, R
   ursa-i9-9960x: Supported benchmark langs: Python, R, JavaScript
   ursa-thinkcentre-m75q: Supported benchmark langs: C++, Java
   


-- 
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: github-unsubscribe@arrow.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [arrow] github-actions[bot] commented on pull request #13975: WIP: ARROW-15693: [Dev] Update crossbow templates to use master or main

Posted by GitBox <gi...@apache.org>.
github-actions[bot] commented on PR #13975:
URL: https://github.com/apache/arrow/pull/13975#issuecomment-1227721738

   https://issues.apache.org/jira/browse/ARROW-15693


-- 
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: github-unsubscribe@arrow.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [arrow] kevingurney commented on pull request #13975: ARROW-15693: [Dev] Update crossbow templates to use master or main

Posted by GitBox <gi...@apache.org>.
kevingurney commented on PR #13975:
URL: https://github.com/apache/arrow/pull/13975#issuecomment-1238247120

   Thanks for the code reviews everyone!
   
   Regarding the performance regressions reported by `ursabot` - these seem a bit surprising. My initial guess would be that they are unrelated to the infrastructural changes in this PR.
   
   Perhaps, we should re-run the benchmarks to make sure these regressions aren't false positives.


-- 
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: github-unsubscribe@arrow.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [arrow] kevingurney commented on a diff in pull request #13975: ARROW-15693: [Dev] Update crossbow templates to use master or main

Posted by GitBox <gi...@apache.org>.
kevingurney commented on code in PR #13975:
URL: https://github.com/apache/arrow/pull/13975#discussion_r959543044


##########
dev/tasks/conda-recipes/azure.clean.yml:
##########
@@ -1,3 +1,5 @@
+{% import 'macros.jinja' as macros with context %}

Review Comment:
   After searching around a bit more - it looks like many of the files in the `/dev/tasks/conda-recipes` directory are missing license headers. It might be worth following up with a future PR to add them.



-- 
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: github-unsubscribe@arrow.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [arrow] raulcd commented on pull request #13975: ARROW-15693: [Dev] Update crossbow templates to use master or main

Posted by GitBox <gi...@apache.org>.
raulcd commented on PR #13975:
URL: https://github.com/apache/arrow/pull/13975#issuecomment-1238366544

   hi @kevingurney I don't think the benchmark issues are related to this PR, I've seen this other PR reporting the same issues:
   https://github.com/apache/arrow/pull/14019#issuecomment-1238363317


-- 
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: github-unsubscribe@arrow.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [arrow] kevingurney commented on pull request #13975: ARROW-15693: [Dev] Update crossbow templates to use master or main

Posted by GitBox <gi...@apache.org>.
kevingurney commented on PR #13975:
URL: https://github.com/apache/arrow/pull/13975#issuecomment-1232107763

   Ok, I just pushed a fix for the linting issues. My apologies again for not noticing them sooner!


-- 
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: github-unsubscribe@arrow.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [arrow] kevingurney commented on pull request #13975: ARROW-15693: [Dev] Update crossbow templates to use master or main

Posted by GitBox <gi...@apache.org>.
kevingurney commented on PR #13975:
URL: https://github.com/apache/arrow/pull/13975#issuecomment-1230474294

   These changes should be ready for review now.
   
   Our apologies for the delay - it took us some time to qualify our changes in a robust way using @lafiona's [queue repository](https://github.com/lafiona/crossbow).
   
   @assignUser - if you have any bandwidth to review these changes, that would be greatly appreciated. Thank you!


-- 
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: github-unsubscribe@arrow.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [arrow] kevingurney commented on a diff in pull request #13975: ARROW-15693: [Dev] Update crossbow templates to use master or main

Posted by GitBox <gi...@apache.org>.
kevingurney commented on code in PR #13975:
URL: https://github.com/apache/arrow/pull/13975#discussion_r959531022


##########
dev/tasks/conda-recipes/azure.clean.yml:
##########
@@ -1,3 +1,5 @@
+{% import 'macros.jinja' as macros with context %}

Review Comment:
   1. You are absolutely right! Importing the macros are not required anymore. This was an oversight on my part - I left this in by accident from a previous version of the code which used a jinja macro instead of a method on the `Target` object to check whether the current branch is the default branch. Since leaving this import statement in didn't cause any obvious issues, I completely forgot to remove it. My apologies! Thank you for catching this!
   
   2. Of course - I will add the License header.



-- 
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: github-unsubscribe@arrow.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [arrow] raulcd commented on pull request #13975: ARROW-15693: [Dev] Update crossbow templates to use master or main

Posted by GitBox <gi...@apache.org>.
raulcd commented on PR #13975:
URL: https://github.com/apache/arrow/pull/13975#issuecomment-1238268978

   @ursabot please benchmark


-- 
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: github-unsubscribe@arrow.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org