[ty] Deploy ecosystem diff to Cloudflare pages#19234
Conversation
|
ea1d49d to
76705d7
Compare
## Summary Add PR comment workflow as a prerequisite for #19234 ## Test Plan Not yet tested. Need to merge this first.
|
a70a11f to
9cd2a0e
Compare
| ignore: | ||
| - build-docker.yml | ||
| - publish-playground.yml | ||
| - ty-ecosystem-analyzer.yaml |
There was a problem hiding this comment.
To be honest, I didn't fully understand what kind of problem this cache-poisoning lint tries to prevent. It felt like it wouldn't be relevant here(?), and then I saw that publish-playground.yml was also excluded here. Maybe I should ask our newest team member at some point 😄
There was a problem hiding this comment.
cache-poisoning mostly tries to prevent dataflows where two conditions hold:
- The workflow has a "publish" trigger (like
release) OR uses a well-known "publishing" action (likecloudflare/wrangler-action) - The workflow contains a separate "cache-aware" action that has caching enabled, e.g.
actions/setup-python.
The basic theory behind it is that (1) identifies publishing behavior, and that publishing behavior should generally be hermetic and not subject to a potentially poisoned cache entry.
OTOH in practice this can be very noisy, as is seen here 😅 -- not all publishing behaviors actually need to be hermetic. But it's hard to assert that in a blanket fashion, so for now zizmor mostly leaves it up to users to manually ignore a cache-poisoning finding that isn't actually relevant to them.
(More generally, none of this would be an issue if GitHub made poisoning the actions cache harder + eliminated trivial pivoting between workflow runs via the cache.)
There's a race condition in ecosystem-analyzer where we get spurious changes if a project changes between computing the old diagnostics and the new ones. This should obviously be fixed (and would also be much faster), but has nothing to do with this PR. |
zanieb
left a comment
There was a problem hiding this comment.
Doesn't this mean that the ecosystem check won't run on user-contributed pull requests because they won't have access to secrets? What's your plan for that?
Oh. I was not aware of that limitation :-| That means I also don't have a plan on how to address this. Would it still work if the workflow is manually triggered by someone with the necessary permissions? I'll do some research tomorrow. |
You can use the |
|
Merging this for now. It should still work on contributor PRs, but will skip deployment to Cloudflare pages. I added an upload step so that |
8827e43 to
200cded
Compare
…re_help * 'main' of https://github.com/astral-sh/ruff: Add simple integration tests for all output formats (astral-sh#19265) [`flake8-return`] Fix false-positive for variables used inside nested functions in `RET504` (astral-sh#18433) [ty] Add a `--quiet` mode (astral-sh#19233) Treat form feed as valid whitespace before a line continuation (astral-sh#19220) [ty] Make `check_file` a salsa query (astral-sh#19255) [ty] Consolidate submodule resolving code between `types.rs` and `ide_support.rs` (astral-sh#19256) [ty] Remove countme from salsa-structs (astral-sh#19257) [ty] Improve and document equivalence for module-literal types (astral-sh#19243) [ty] Optimize protocol subtyping by removing expensive and unnecessary equivalence check from the top of `Type::has_relation_to()` (astral-sh#19230) [ty] Ecosystem analyzer: parallelize, fix race condition (astral-sh#19252) [ty] Add completion kind to playground (astral-sh#19251) [ty] Deploy ecosystem diff to Cloudflare pages (astral-sh#19234) [ty] Add semantic token provider to playground (astral-sh#19232)
Summary
Changes the ecosystem-analyzer workflow to deploy the diff to Cloudflare pages and post a link in the PR. Also adds a summary statistics to that PR comment.
Test Plan
The comment below: #19234 (comment). I previously had some dummy changes on this PR to see a non-zero diff. And I didn't reapply the label after I reverted that change, such that it's still visible for reviewers.