Skip to content

Parallelize CI Jobs#263

Merged
ThomasLaPiana merged 21 commits into
mainfrom
ps/parallelize-ci
Dec 10, 2021
Merged

Parallelize CI Jobs#263
ThomasLaPiana merged 21 commits into
mainfrom
ps/parallelize-ci

Conversation

@PSalant726
Copy link
Copy Markdown
Contributor

@PSalant726 PSalant726 commented Dec 1, 2021

Closes #228

Code Changes

  • Parallelize the CI pipeline

Steps to Confirm

  • Review pipeline config
  • Review pipeline output

Pre-Merge Checklist

  • All CI Pipelines Succeeded

Description Of Changes

Attempts to create and store a fidesctl Docker image artifact that is then shared between CI jobs within a single workflow.

@PSalant726 PSalant726 changed the title First pass at parallelized tests Parallelize CI Jobs Dec 1, 2021
@PSalant726 PSalant726 force-pushed the ps/parallelize-ci branch 7 times, most recently from 28004b8 to 0f2b8d9 Compare December 1, 2021 16:55
@PSalant726 PSalant726 force-pushed the ps/parallelize-ci branch 2 times, most recently from 54cebad to 64bcfcc Compare December 1, 2021 22:14
@PSalant726 PSalant726 marked this pull request as ready for review December 2, 2021 23:41
Comment thread .github/workflows/pr_checks.yml
Comment thread .github/workflows/pr_checks.yml Outdated
Comment thread .github/workflows/pr_checks.yml Outdated
Comment thread .github/workflows/pr_checks.yml Outdated
@PSalant726 PSalant726 self-assigned this Dec 2, 2021
@PSalant726 PSalant726 added dev experience Targets the developer experience enhancement help wanted Extra attention is needed high-risk This issue suggests changes that have a high-probability of breaking existing code testing labels Dec 2, 2021
@ThomasLaPiana ThomasLaPiana self-assigned this Dec 6, 2021
@ThomasLaPiana
Copy link
Copy Markdown
Contributor

im very confused at this point...it is loading the cache from the image but not using it for some reason?

https://github.com/ethyca/fides/runs/4438492231?check_suite_focus=true#step:5:62

Copy link
Copy Markdown
Contributor

@ThomasLaPiana ThomasLaPiana left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Had to make some changes but got it working

@ThomasLaPiana
Copy link
Copy Markdown
Contributor

@PSalant726 can you give this a quick review? i finally got it! but want to make sure what I did isn't too crazy

@ThomasLaPiana
Copy link
Copy Markdown
Contributor

i merged main and tested the new pre-commit checks, they work as well

Copy link
Copy Markdown
Contributor Author

@PSalant726 PSalant726 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@ThomasLaPiana Nice Docker-fu! Just some minor things. Only the first comment is a blocker, but I can't "request changes" since I'm the original author.

Comment thread .github/workflows/pr_checks.yml Outdated
Comment thread .github/workflows/pr_checks.yml Outdated
Comment thread .github/workflows/pr_checks.yml Outdated
Comment thread .github/workflows/pr_checks.yml
Comment thread .pre-commit-config.yaml
Comment thread Makefile Outdated
@ThomasLaPiana ThomasLaPiana merged commit 9f80a69 into main Dec 10, 2021
@ThomasLaPiana ThomasLaPiana deleted the ps/parallelize-ci branch December 10, 2021 17:26
ThomasLaPiana pushed a commit that referenced this pull request Aug 17, 2022
Adding ability to download privacy requests as CSV
ThomasLaPiana pushed a commit that referenced this pull request Sep 26, 2022
Adding ability to download privacy requests as CSV
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

dev experience Targets the developer experience help wanted Extra attention is needed high-risk This issue suggests changes that have a high-probability of breaking existing code testing

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Parallelize CI Jobs

2 participants