Add action to push screenshots to conda store

[kcpevey]

Fixes conda-incubator/conda-store#570

Description

This workflow collects screenshots which have stored as artifacts in test.yml and opens a PR on conda-store to update the docs.

This pull request:

• adds gh action to push screenshots to conda-store

Pull request checklist

• Did you test this change locally?
• Did you update the documentation (if required)?
• Did you add/update relevant tests for this change (if required)?

Additional information

We originally discussed triggering this from a comment on a PR. I discovered that the only Action trigger, issue_comment, would trigger this to run for every comment on every issue. I feel like that is a lot of wasted compute even if the first thing it did was check to see if its a PR and shut it down. For this reason, I've decided to trigger on label instead.

# There are two ways to trigger this workflow:
# 1. Trigger by adding the `update_screenshots` label. 
#    The workflow will run immediately after you add the label. It is
#    also expecting artifacts which have been generated in test.yml. 
#    Therefore, this cannot be triggered until test.yml is complete. 
# 2. Trigger by workflow dispatch.
#    Specify the PR number with the artifacts you'd like to use.
#    Again, the test.yml workflow must be complete.
#
# If a PR has already been opened on the conda-store repo from this
# action, a rerun of this action will attempt to update the PR, not
# create a new one. 

I looked at triggering on the completion of test.yml, but that would trigger the workflow off of main and we'd be forced to use the "latest" generated artifacts, rather than PR-specific.

This will require a PAT to open up the PR on the conda-store side. It is currently looking for secrets.PAT.

[kcpevey]

I've tested this approach on dummy repos but this will still take me some tuning to get everything right in place.

TODO:

• get secrets sorted out
• add more screenshots to playwright to cover what's already in conda-store
  • make sure the words of the doc aren't specific to the old screenshot (i.e. has the doc been updated to the new UI)
• sort out where these need to land (in the new docs folder or overwrite the old screenshots?)

[kcpevey]

@pavithraes it looks like a lot of the images in /docusaurus-docs/conda-store-ui/images have red boxes around the area of interest. We will lose those if we let playwright generate the images. Is that ok, or should we avoid updating those images with playwright?

[kcpevey]

Ok, this thing is back to passing CI 🎉 Now I need to double check the images.

[kcpevey]

I suspect this is failing due to a bad token. We were originally using @jaimergp 's PAT. Can you please regenerate and add it as a secret named CONDA_STORE_SYNC in this repo? It needs to be able to open a PR on https://github.com/conda-incubator/conda-store

[gabalafou]

I suspect this is failing due to a bad token. We were originally using @jaimergp 's PAT.

I have a question about this. Is this going to create any maintenance issues in the future? Is this the most future-proof approach we can take?

[kcpevey]

I have a question about this. Is this going to create any maintenance issues in the future? Is this the most future-proof approach we can take?

Because we are opening a PR on another repo, we need credentials. In other projects, we have github service accounts where we share credentials (so basically internal "bot" users). Since this would have to be a user in the conda-incubator org, we didn't feel that was appropriate and decided to go with personal credentials. Happy to explore any other ideas though!

[github-actions]

A PR to conda-store has been opened with updated screenshots - conda-incubator/conda-store#731

[jaimergp]

Fixed.

[kcpevey]

I looked through the built docs on the other PR. I need to make a few minor tweaks to some of the image clipping.

[github-actions]

A PR to conda-store has been opened with updated screenshots - conda-incubator/conda-store#731

[gabalafou]

I would still like to see this commit message updated

Suggested change

	git commit -m "commit all changes"
	git commit -m "🤖 conda-store-ui/update_screenshots"

[kcpevey]

ah. Sorry, I changed this line instead when you requested the first time.

Ok... little bit of a wild goose chase, but I think I got it. The commit-message below (line 92) is being ignored and THIS line is what's being used.

[gabalafou]

Yay, this is coming together. Sorry to insist, but I really want us to update the commit message in the workflow

[kcpevey]

Unfortunately I had the images looking great on my local setup (which is running the same conda-store version as CI) but the images I generated locally differ from CI. The images that require clipping are "off" by an unacceptable amount.

[gabalafou]

Sorry to possibly rock the boat here, but I am starting to wonder if we should abandon this PR.

Similar to CI, I also had iffy results running this locally. For example, the following screenshot is not ready to be published straight to the docs:

It's to be expected that there will be differences between the screenshots taken on CI and those taken locally. This came up when I started working on JupyterLab, which has a number of snapshot tests, which compares saved screenshots taken on a previous run and committed to the repo with screenshots taken on the current run. If you did anything that changed the UI, you couldn't update the saved and committed screenshots from your local computer; you had to upload your changes in a PR and then ask a GitHub robot to attach updated screenshots to your PR.

I don't want us to fall into a sunk cost fallacy here. We have spent a lot of time to get this PR close to the finish line, but considering the work left to do and the work it may demand in the future, have we already tipped past the value this PR potentially provides?

[trallard]

I am leaning towards the same view as @gabalafou, this PR has lagged a lot more than I would have liked both in time and effort, and I am worried that it will add a non-trivial amount of maintenance and upkeep work.

[trallard]

Please refer to the original issue.
At this point and after 10 months of this PR being WIP I think the best is to close without merging.

[kcpevey]

Apologies I couldn't get this across the finish line. I just didn't have enough consistent hours and there was too much breakage every time I returned. I suspect this means that maintenance burden would be significant as well (having to change the crop extents slightly all the time). Closing seems reasonable.