-
-
Notifications
You must be signed in to change notification settings - Fork 25.9k
[WIP] Add [pypy] test marker for circle-ci #13023
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
Conversation
It looks like circle didn't run on this pr at all. Config broken? |
I have enabled Circle CI in my fork repo, would that cause any problem? |
Possibly? What is its status?
|
It did not run, says I need to choose a plan for Mac OS to run the job, I dont know why is that necessary. |
63bff83
to
d48196e
Compare
After I disabled Circle CI in my repo, it runs here. Now the script needs an API key to trigger the job, can someone with CI access setup environment key |
We cannot pass secret credentials to CI jobs run in PRs, right? Otherwise that would be a security issue: a user could submit a PR to change the CI script to display the secret token on stdout to retrieve it. |
Yes, looks like this might a dead end due to security concerns (cf https://circleci.com/blog/managing-secrets-when-you-have-pull-requests-from-outside-contributors/). Hmm, I guess potentially an API Token could be used to do anything on Circle CI, not just re-trigger jobs? At the same time, the alternative proposed in #13014 (comment) of having the PyPy job always run but not do anything unless on master or if this commit flag is detected is not great either. Since green CI would not actually mean that PyPy CI passed. Any ideas what we could do? Maybe it's an opportunity to play with Github Actions? Since we can start that Circle CI job, just not from circle CI as that would expose the token... |
we already do the "run the job but return 0" on the |
I find that situation not ideal, basically CI status is getting less useful. For deploy, we don't really care. Here, it would make contributors think that their code passes PyPy CI while it's actually not the case. Though, I agree that it's probably the simplest solution in the end. I'm just wondering if there is another simple way around that. |
Are we limited to only Circle CI for running pypy tests? how about other CI vendors? Maybe we could run the cron job in Circle CI and on-demand runs elsewhere if they are conducive for our use case. |
Travis CI conditional job should work fine for pypy test marker |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Small comment:
|
||
commit_msg=$(git log --format=%B -n 1 $CIRCLE_SHA1) | ||
|
||
if [[ "$commit_msg" =~ "[pypy]" ]] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
May I suggest to use "[ci pypy]"
instead to make it explicit that this is about Continuous Integration?
@nixphix could you try to move this PR to azure pipelines or github actions to workaround the limitations of Circle CI? github actions in particular does support conditions expressions involving commit message (we do it for the BTW, I have modified the configuration of the PyPy build to use conda-forge to setup the PyPy env (instead of docker). |
Closing since we now run |
Reference Issues/PRs
Fixes #13014
What does this implement/fix? Explain your changes.
Circle CI lint job will check for
[pypy]
marker in commit message and if available then it would trigger pypy test job.