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

ID-377 Add drshub consumer test url resolution #7142

Conversation

Ghost-in-a-Jar
Copy link
Contributor

@Ghost-in-a-Jar Ghost-in-a-Jar commented May 18, 2023

Added pact github actions workflows from leo.

Added consumer test for drshub url resolution.

@Ghost-in-a-Jar Ghost-in-a-Jar requested a review from a team as a code owner May 18, 2023 17:19
repo: broadinstitute/terra-github-workflows
ref: refs/heads/main
token: ${{ secrets.BROADBOT_TOKEN }} # github token for access to kick off a job in the private repo
inputs: '{ "pacticipant": "cromwell-consumer", "version": "${{ needs.init-github-context.outputs.repo-version }}" }'
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This is all copied straight from leo's workflows with 'leo' -> 'cromwell'

@Ghost-in-a-Jar Ghost-in-a-Jar force-pushed the ID-377-add-drshub-consumer-test-url-resolution branch from d1ce835 to 54d5fe2 Compare May 18, 2023 17:28

can-i-deploy:
runs-on: ubuntu-latest
if: ${{ needs.init-github-context.outputs.fork == 'false' }}
Copy link
Contributor

Choose a reason for hiding this comment

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

You can disable can-i-deploy and publish the cromwell contract to pact broker if you like. Later, when your drshub provider test in this PR is done you can add a webhook (essentially just like https://github.com/broadinstitute/sam/blob/develop/.github/workflows/verify_consumer_pacts.yml and you can reenable can-i-deploy again. This is just a suggestion.

Suggested change
if: ${{ needs.init-github-context.outputs.fork == 'false' }}
if: ${{ needs.init-github-context.outputs.fork == 'false' && false }}

Copy link
Contributor Author

Choose a reason for hiding this comment

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

That sounds good thanks

Copy link
Contributor

@THWiseman THWiseman left a comment

Choose a reason for hiding this comment

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

LGTM. This is awesome to have!

@Ghost-in-a-Jar Ghost-in-a-Jar force-pushed the ID-377-add-drshub-consumer-test-url-resolution branch from 54d5fe2 to 476aaff Compare May 22, 2023 19:59
@Ghost-in-a-Jar Ghost-in-a-Jar merged commit 698bb29 into ID-377-add-cromwell-pact May 23, 2023
@Ghost-in-a-Jar Ghost-in-a-Jar deleted the ID-377-add-drshub-consumer-test-url-resolution branch May 23, 2023 14:42
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants