Skip to content

Ci shared workflows for sub project#6025

Merged
jeff-dude merged 6 commits into
mainfrom
ci-shared-workflows
May 29, 2024
Merged

Ci shared workflows for sub project#6025
jeff-dude merged 6 commits into
mainfrom
ci-shared-workflows

Conversation

@aalan3

@aalan3 aalan3 commented May 29, 2024

Copy link
Copy Markdown
Contributor

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.

@aalan3 aalan3 requested review from 0xRobin, couralex6 and jeff-dude May 29, 2024 12:34
@0xRobin

0xRobin commented May 29, 2024

Copy link
Copy Markdown
Contributor

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

aalan3 commented May 29, 2024

Copy link
Copy Markdown
Contributor Author

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

0xRobin commented May 29, 2024

Copy link
Copy Markdown
Contributor

@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

aalan3 commented May 29, 2024

Copy link
Copy Markdown
Contributor Author

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

@jeff-dude

Copy link
Copy Markdown
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

aalan3 commented May 29, 2024

Copy link
Copy Markdown
Contributor Author

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
Copy Markdown
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
@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
Comment thread .github/workflows/spellbook.yml
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