You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@hamilton.apache.org by "jernejfrank (via GitHub)" <gi...@apache.org> on 2025/06/14 23:13:41 UTC
[PR] Migrate to GitHub acitions [hamilton]
jernejfrank opened a new pull request, #1342:
URL: https://github.com/apache/hamilton/pull/1342
Addressing #1313 and uses uv.
I don't have access to CircleCI to see the settings there (I assume the config with jobs and worklof is only part of it, since there's no mention of OS inside), would be good to let me know if anything else needs to be configured for the github actions workflow.
## Changes
- replaced circleci config with github actions
- minor linting and bugfixes
## How I tested this
- Need to invoke CI from main repo
## Notes
- Fail-fast is disabled for now, so that I can see all of the tests in the matrix failing
- Not sure if we need to test on all platforms
- Adds the option to run the tests manually through the actions tab
## Checklist
- [x] PR has an informative and human-readable title (this will be pulled into the release notes)
- [x] Changes are limited to a single goal (no scope creep)
- [x] Code passed the pre-commit check & code is left cleaner/nicer than when first encountered.
- [ ] Any _change_ in functionality is tested
- [ ] New functions are documented (with a description, list of inputs, and expected output)
- [ ] Placeholder code is flagged / future TODOs are captured in comments
- [ ] Project documentation has been updated if adding/changing functionality.
--
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: notifications-unsubscribe@hamilton.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Migrate to GitHub acitions [hamilton]
Posted by "skrawcz (via GitHub)" <gi...@apache.org>.
skrawcz commented on PR #1342:
URL: https://github.com/apache/hamilton/pull/1342#issuecomment-2973465826
> I don't have access to CircleCI to see the settings there (I assume the config with jobs and worklof is only part of it, since there's no mention of OS inside), would be good to let me know if anything else needs to be configured for the github actions workflow.
@jernejfrank you can see everything in the .circleci yaml and what it references. It's similar to github workflows, but we have more flexibility on when a workflow is triggered with the github way. So shouldn't be hard to replicate.
--
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: notifications-unsubscribe@hamilton.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Migrate to GitHub acitions [hamilton]
Posted by "skrawcz (via GitHub)" <gi...@apache.org>.
skrawcz commented on code in PR #1342:
URL: https://github.com/apache/hamilton/pull/1342#discussion_r2157941572
##########
.github/workflows/hamilton-main.yml:
##########
@@ -0,0 +1,163 @@
+name: Unit Tests
+
+on:
+ workflow_dispatch:
+
+ pull_request:
+ branches:
+ - main
+ paths:
+ - '.github/**'
+ - 'hamilton/**'
+ - 'plugin_tests/**'
+ - 'tests/**'
+ - 'pyproject.toml'
+
+jobs:
+ test:
+ name: "Unit Tests"
+ runs-on: ${{ matrix.os }}
+ strategy:
+ fail-fast: false # will change this to true at the end, but want to see tests failing on all use cases
Review Comment:
actually I think we want this to be false always.
--
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: notifications-unsubscribe@hamilton.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Migrate to GitHub acitions [hamilton]
Posted by "jernejfrank (via GitHub)" <gi...@apache.org>.
jernejfrank merged PR #1342:
URL: https://github.com/apache/hamilton/pull/1342
--
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: notifications-unsubscribe@hamilton.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Migrate to GitHub acitions [hamilton]
Posted by "jernejfrank (via GitHub)" <gi...@apache.org>.
jernejfrank commented on PR #1342:
URL: https://github.com/apache/hamilton/pull/1342#issuecomment-2981950666
Right, so SDK was passing but now the CI dissapeared. Any thoughts @skrawcz?
I am also thinking of limiting it to Linux (for now) since I saw that windows has some issues with package installation when using `uv` since it does a better job at finding problems than `pip`.
--
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: notifications-unsubscribe@hamilton.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Migrate to GitHub acitions [hamilton]
Posted by "jernejfrank (via GitHub)" <gi...@apache.org>.
jernejfrank commented on PR #1342:
URL: https://github.com/apache/hamilton/pull/1342#issuecomment-2989224088
@skrawcz ok, got the tests sorted out and did a rebase. Let me know if you're happy with squash merge or need me to organize the commits :)
--
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: notifications-unsubscribe@hamilton.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Migrate to GitHub acitions [hamilton]
Posted by "skrawcz (via GitHub)" <gi...@apache.org>.
skrawcz commented on code in PR #1342:
URL: https://github.com/apache/hamilton/pull/1342#discussion_r2147414159
##########
pyproject.toml:
##########
@@ -27,7 +27,7 @@ classifiers = [
"Programming Language :: Python :: 3.12",
]
dependencies = [
- "numpy",
+ "numpy<2",
Review Comment:
this isn't good since 2.0 has been out for a while.
##########
pyproject.toml:
##########
@@ -58,7 +57,8 @@ docs = [
# required for all the plugins
"dlt",
# furo -- install from main for now until the next release is out:
- "furo @ git+https://github.com/pradyunsg/furo@main",
+ "furo",
Review Comment:
+1
--
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: notifications-unsubscribe@hamilton.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Migrate to GitHub acitions [hamilton]
Posted by "skrawcz (via GitHub)" <gi...@apache.org>.
skrawcz commented on PR #1342:
URL: https://github.com/apache/hamilton/pull/1342#issuecomment-2973467633
The SDK tests aren't passing because spark changed things. I think we just need to update the tests...
--
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: notifications-unsubscribe@hamilton.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Migrate to GitHub acitions [hamilton]
Posted by "jernejfrank (via GitHub)" <gi...@apache.org>.
jernejfrank commented on PR #1342:
URL: https://github.com/apache/hamilton/pull/1342#issuecomment-2973558317
> > I don't have access to CircleCI to see the settings there (I assume the config with jobs and worklof is only part of it, since there's no mention of OS inside), would be good to let me know if anything else needs to be configured for the github actions workflow.
>
> @jernejfrank you can see everything in the .circleci yaml and what it references. It's similar to github workflows, but we have more flexibility on when a workflow is triggered with the github way. So shouldn't be hard to replicate.
Ah ok, then all is migrated! I thought there might be something more since the .circleci yaml doesn't mention anything about OS to test on, etc..
Also added this to the notes of the pr:
- some test (mostly plugins) were running only on specific python versions and run now on all supported python versions of the plugin package:
- Ray, Dask, PySpark 3.9-3.12
- pre-commit 3.8-3.12 (sometimes important to spot things like needing `from __future__ import annotations`)
- pandas, polars, narwhals 3.8-3.12
Will be fixing broken tests, but may take a while!
--
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: notifications-unsubscribe@hamilton.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Migrate to GitHub acitions [hamilton]
Posted by "skrawcz (via GitHub)" <gi...@apache.org>.
skrawcz commented on code in PR #1342:
URL: https://github.com/apache/hamilton/pull/1342#discussion_r2157941392
##########
.github/workflows/hamilton-main.yml:
##########
@@ -0,0 +1,163 @@
+name: Unit Tests
+
+on:
+ workflow_dispatch:
+
+ pull_request:
+ branches:
+ - main
+ paths:
+ - '.github/**'
+ - 'hamilton/**'
+ - 'plugin_tests/**'
+ - 'tests/**'
+ - 'pyproject.toml'
+
+jobs:
+ test:
+ name: "Unit Tests"
+ runs-on: ${{ matrix.os }}
+ strategy:
+ fail-fast: false # will change this to true at the end, but want to see tests failing on all use cases
Review Comment:
need to flip 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: notifications-unsubscribe@hamilton.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Migrate to GitHub acitions [hamilton]
Posted by "jernejfrank (via GitHub)" <gi...@apache.org>.
jernejfrank commented on code in PR #1342:
URL: https://github.com/apache/hamilton/pull/1342#discussion_r2147479733
##########
pyproject.toml:
##########
@@ -27,7 +27,7 @@ classifiers = [
"Programming Language :: Python :: 3.12",
]
dependencies = [
- "numpy",
+ "numpy<2",
Review Comment:
Forgot to remove when debugging some other dependencies issues. Removed 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: notifications-unsubscribe@hamilton.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Migrate to GitHub acitions [hamilton]
Posted by "skrawcz (via GitHub)" <gi...@apache.org>.
skrawcz commented on PR #1342:
URL: https://github.com/apache/hamilton/pull/1342#issuecomment-2982277400
> Right, so SDK was passing but now the CI dissapeared. Any thoughts @skrawcz?
Can you rebase against main?
> I am also thinking of limiting it to Linux (for now) since I saw that windows has some issues with package installation when using uv since it does a better job at finding problems than pip.
That's fine I think.
--
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: notifications-unsubscribe@hamilton.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org