-
-
Notifications
You must be signed in to change notification settings - Fork 84
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
chore: add CI validation for pull requests #217
base: master
Are you sure you want to change the base?
Conversation
@aeneasr this would be my proposal to have CI run on SDK PRs (e.g. updating dependencies, etc.). Alternatively, we could run one workflow per language/client combination, but that got very bloated and is not very clearly usable, IMO. With 5 clients à 10 languages, that would be 50 jobs per PR :D Are you okay with the format (e.g. one workflow for each language)? If so, I'll make this ready to use after my vacation. |
spec/client/latest
Outdated
@@ -0,0 +1 @@ | |||
v1.1.9 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
These will be overridden by the SDK publish task, but need to be added manually here so the tests of this PR pass.
Next step would be, to update the release pipeline to use GHA as well. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
A bit difficult to review because there are many changes to the cwd. Should we push another kratos release for the SDK to fix first?
.github/workflows/ci.yaml
Outdated
jobs: | ||
test: | ||
runs-on: ubuntu-latest | ||
container: oryd/sdk:v0.0.51 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We need to keep this in sync now as well?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, we do. But I do intend to clean up the Circle CI pipeline soon-ish as well.
And ideally, I'd like to see if we can split up all these SDK generation processes into different workflows (per language), that use their own base images. But not sure if there are many benefits to that yet.
@aeneasr updated the branch. PTAL. |
It's failing :( |
Please excuse my barging in. I was just looking into Swift language support (#273), and this might actually block that effort. Apple's official Swift builds come in Ubuntu but not Debian per se. So I'd begun to surgically retrofit the appropriate libraries into the Dockerfile. Gradually applying an embarrassing series of tricks. Then I started noticing evidence of similar tricks, duplication, and unexpected side-effects already present — in the stanzas for other languages that others had added before me. It occurs to me that the project might be, essentially… inside-out? Apart from the WETness of it all — setting aside the distribution of concerns across many repeating parallel lists. By now this unwieldy, monolithic container has begun to undermine its own ability to test the final product for use in broadly heterogenous environments. Might it help to leverage multi-stage builds here? Something like the following crude scaffolding, giving each language its own native container environment. You'd still have one pass for each client SDK, but then all languages (and others, and dialects to be easily added later) would still build/test within a single CI context that can pass or fail and block a release. As a bonus, I think BuildKit would nicely parallelize the bulk of it? Anyway: it's a thought that I had while struggling to accommodate Swift and Ubuntu dependencies in the current structure.
|
@jonct thanks for the input! This is very valuable.
That could work. One reason for the current setup is that failing builds can easily be debugged on a local environment (in theory, there are issues with the Dart build and M1 macs right now), as you can just pull the image and rerun with the same inputs. I am guessing that could still be easily achieved with a multi-stage build, though. I'd have two questions that you may be able to answer:
I was actually thinking about moving the whole workflow to individual steps as GitHub actions jobs. The individual language builds don't have much in common, and are defined as individual functions in This PR serves as a playground to explore those ideas, but due to time constraints, it has gone a bit stale. |
Hi there! Thanks; great questions.
I don't know exactly how any given builder tool handles dependencies. If they're smart enough to parallelize stages until they're needed by other stages, they might also be smart enough to skip stages that they don't need for a given named target stage. In which case e.g. But overall, you'd have a giant reduction in individual package management. The full build might just become quick enough that it doesn't matter? The burden of caching ends up at the container registry level, and might be more effective that way.
Oh. Well, that could open up other options as you've suggested. And as you've noted, you could end up with dozens of CI jobs per commit. I suppose this is a question of what you're aiming to test here: whether 1) a newly modified API spec breaks one or more platforms and should thus be reviewed by humans prior to official release, or if 2) a given language's sample project just needs to be updated to meet the revised API spec, so that one language's binding is slightly delayed relative to the others. I'm not sure which is best for your internal view of the project. But I'll register my vote that any "tests" of a given language or platform SDK should be conducted atop a native vanilla baseline, accepted as representative of that language or platform. Thanks for all that you do! The Ory philosophy is filling a great need here, and my team is grateful for your efforts. 🥳 |
Another option, BTW: use the repositories for each language SDK to house its own sample project and testing rigs. Have this project submit generated SDKs as pull requests to the respective projects. |
Related Issue or Design Document
Checklist
If this pull request addresses a security vulnerability,
I confirm that I got approval (please contact [email protected]) from the maintainers to push the changes.
Further comments