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

Ci shared workflows for sub project #6025

Merged
merged 6 commits into from
May 29, 2024
Merged

Ci shared workflows for sub project #6025

merged 6 commits into from
May 29, 2024

Conversation

aalan3
Copy link
Contributor

@aalan3 aalan3 commented May 29, 2024

This PR updates the CI flow to accommodate for more sub projects.

Dbt slim ci has been replaced with a shared workflow that does the same thing. Each subproject then needs to have their own workflow files that 1) ignores/selects the files that are relevant and 2) calls the shared workflow.

As a result of this update the main spellbook CI will no longer run if nothing is changed in the main project, which will result in faster CI runs.

@0xRobin
Copy link
Collaborator

0xRobin commented May 29, 2024

Thoughts on putting all the dbt sub-projects into 1 directory instead of them all living in the root dir?
Will be better to keep the overview when we scale to multiples of them.

@aalan3
Copy link
Contributor Author

aalan3 commented May 29, 2024

I don't have a super strong opinion on this but we making that change in the future should be straightforward. I think it depends a bit on what the root dir looks like later

@0xRobin
Copy link
Collaborator

0xRobin commented May 29, 2024

@aalan3 agree it comes down to preference/opinion. I do think having both the core spellbook dbt directories (seeds/scripts/macros/...) and the subproject directories (tokens/ spellbook-daily/..) scattered on the same dir level will be confusing for new contributors.

@aalan3
Copy link
Contributor Author

aalan3 commented May 29, 2024

Yeah that makes sense, I think the seeds/scripts/macros should go into something like shared eventually.

@jeff-dude
Copy link
Member

i think we should move forward with the directory structure organization now while we're in the weeds here. it also helps with a few suggestions to the code i have in this PR.

  • root directory
    • spellbook_projects
      • spellbook_daily
        • dbt_project.yml
        • profiles.yml
        • packages.yml
        • package_lock.yml
        • models/
        • tests/
        • seeds/
      • tokens
        • dbt_project.yml
        • profiles.yml
        • packages.yml
        • package_lock.yml
        • models/
        • tests/
        • seeds/
      • dex
        • dbt_project.yml
        • profiles.yml
        • packages.yml
        • package_lock.yml
        • models/
        • tests/
        • seeds/
      • ...future sub projects
      • macros/
      • sources/
  • docs/
  • scripts/
  • .github/
  • ....and others needed to remain here

we haven't tried moving tests/seeds into subprojects yet, but that is because we've only done tokens and there weren't any to move with the models. i think it'll make sense to have those project specific, but we can be lenient to move back a level with macros/sources if it demands it.

then with this structure, we can simplify the CI here to use paths: and specify one subproject path, rather than path-ignores on all other subprojects. that could grow out of hand potentially.

maybe this means we need to move the two PRs back into one, to make it all work? or at least fast follow to merge one after the other and be prepared to ensure it all works.

@aalan3
Copy link
Contributor Author

aalan3 commented May 29, 2024

I don't think we should do this now and in the context of this PR @jeff-dude. Lets merge this and test the new sub project and after that we can move things around in the repo if we want. Right now the benefit to CI structure is not that much since we only have 2 sub projects.

@jeff-dude
Copy link
Member

I don't think we should do this now and in the context of this PR @jeff-dude. Lets merge this and test the new sub project and after that we can move things around in the repo if we want. Right now the benefit to CI structure is not that much since we only have 2 sub projects.

makes sense, but let's remember this comment in future cleanup

@jeff-dude jeff-dude merged commit 7510f06 into main May 29, 2024
3 checks passed
@jeff-dude jeff-dude deleted the ci-shared-workflows branch May 29, 2024 15:13
@github-actions github-actions bot locked and limited conversation to collaborators May 29, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants