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

Update example apps #13

Merged
merged 8 commits into from
Aug 30, 2024
Merged

Update example apps #13

merged 8 commits into from
Aug 30, 2024

Conversation

sfc-gh-pjafari
Copy link
Contributor

@sfc-gh-pjafari sfc-gh-pjafari commented Aug 29, 2024

  • add missing readme files
  • remove "create security integration" step from spcs example as it's no longer needed. See jira ticker

@@ -1,5 +1,4 @@
set -e
snow sql -f "prepare/spcs_setup.sql"

Choose a reason for hiding this comment

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

why are we removing this? Maybe some context would help.

Copy link
Contributor Author

@sfc-gh-pjafari sfc-gh-pjafari Aug 29, 2024

Choose a reason for hiding this comment

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

It's no longer needed. updated PR description with Jira ticket :)
https://snowflakecomputing.atlassian.net/browse/SNOW-1527720

Comment on lines +115 to +118
if [ "$ret" = 5 ]; then
echo "No tests collected."
exit 0
fi

Choose a reason for hiding this comment

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

curious: how did you run into this?

Copy link
Contributor Author

@sfc-gh-pjafari sfc-gh-pjafari Aug 29, 2024

Choose a reason for hiding this comment

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

good question :D not all sample projects have python code/python tests. by default, pytest exits with error code 5 if that's the case.
The action is still not passing though so I gotta look into it more (it worked when I tested locally).

Choose a reason for hiding this comment

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

I see, the workflow is failing. I'm a bit rusty, does GH pick up the new workflow from the PR, or would you have to merge this first? If it's running the version from main, your fix won't take effect yet...

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It's running the ci from this PR, I can see my change in the action logs.

Copy link
Contributor

Choose a reason for hiding this comment

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

Also, the CI is known to be broken right now. So no worries about failing runs at the moment, just run the tests locally and confirm they're passing before merge.

@sfc-gh-pjafari sfc-gh-pjafari merged commit f2a488b into main Aug 30, 2024
3 of 4 checks passed
@sfc-gh-pjafari sfc-gh-pjafari deleted the pj-example-update branch August 30, 2024 15:24
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.

4 participants