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

index-dot-shaka-player-demo.appspot.com broken #4074

Closed
avelad opened this issue Mar 29, 2022 · 7 comments · Fixed by #4116, #4119, #4122, #4126 or #4130
Closed

index-dot-shaka-player-demo.appspot.com broken #4074

avelad opened this issue Mar 29, 2022 · 7 comments · Fixed by #4116, #4119, #4122, #4126 or #4130
Assignees
Labels
component: demo page The issue is in the demo page; does not affect production applications priority: P2 Smaller impact or easy workaround status: archived Archived and locked; will not be updated type: bug Something isn't working correctly
Milestone

Comments

@avelad
Copy link
Member

avelad commented Mar 29, 2022

Have you read the FAQ and checked for duplicate open issues? Yes

What did you do?
Open https://index-dot-shaka-player-demo.appspot.com/

What did you expect to happen?
v3 versions are shown

What actually happened?
v3 versions are missing

@avelad avelad added the type: bug Something isn't working correctly label Mar 29, 2022
@github-actions github-actions bot added this to the v3.3 milestone Mar 29, 2022
@joeyparrish
Copy link
Member

Interesting... that looks like test data we use when running the index app locally. The API to query app versions must be failing.

That code is quite old, and we could probably replace the query for appspot versions with a query on GitHub tags instead.

@joeyparrish joeyparrish added the priority: P2 Smaller impact or easy workaround label Mar 29, 2022
@joeyparrish joeyparrish self-assigned this Mar 29, 2022
@joeyparrish joeyparrish added the component: demo page The issue is in the demo page; does not affect production applications label Mar 29, 2022
theodab added a commit to theodab/shaka-player that referenced this issue Apr 5, 2022
These default versions, originally meant for testing, were showing
up in the actual site as it deployed. We're not really doing testing
on these scripts now that they are GitHub actions, so there's no harm
in removing that clause entirely.

Closes shaka-project#4074
theodab added a commit to theodab/shaka-player that referenced this issue Apr 11, 2022
The script for deploying the demo version index page has a function
that chooses which versions to display. This script has a special
mode meant for testing that shows a default list of demo versions.
This mode was only supposed to trigger when run locally, but the
check did not work properly, so the demo version index was deploying
with a very out-of-date list of versions.
This fixes that check.

Closes shaka-project#4074
theodab added a commit that referenced this issue Apr 11, 2022
The script for deploying the demo version index page has a function
that chooses which versions to display. This script has a special
mode meant for testing that shows a default list of demo versions.
This mode was only supposed to trigger when run locally, but the
check did not work properly, so the demo version index was deploying
with a very out-of-date list of versions.
This fixes that check.

Closes #4074

<!--
Please remember to:

1. Use Conventional Commits syntax (fix: ..., feat: ..., etc.) in commits and
   PR title (https://www.conventionalcommits.org/)
2. Tag any related or fixed issues ("Issue #123", "Closes #420")
3. Sign the Google CLA if you haven't (https://cla.developers.google.com)

You may delete this comment from the PR description.
-->
@joeyparrish
Copy link
Member

The fix in PR #4116 appears to have an issue. I'm now seeing:

Internal Server Error

The server encountered an internal error and was unable to complete your request. Either the server is overloaded or there is an error in the application.

I'll reopen the issue for now.

@joeyparrish joeyparrish reopened this Apr 12, 2022
@joeyparrish joeyparrish changed the title https://index-dot-shaka-player-demo.appspot.com/ don't include V3 versions index-dot-shaka-player-demo.appspot.com broken Apr 12, 2022
@joeyparrish
Copy link
Member

The call to google.appengine.api.modules.modules.get_versions() is ultimately failing with AssertionError: No api proxy found for service "modules". This worked fine with the now-deprecated Python 2 engine, and it's not clear that there's a functioning equivalent in the new runtime. (Or maybe there is, but it requires the setup of additional services or something.)

Since the Google Cloud docs are unhelpful on this, I suggest we just stop depending on this app-engine-specific code at all. Back when the index service was launched, it was its own thing, and looking up appspot subdomains from inside appspot made sense.

Now that we have GitHub Actions, there's no reason for any dynamic code in this thing at all. It could be purely static content, regenerated and redeployed each time a release is tagged on GitHub.

I think that's the way forward. It's also independent of Google Cloud. Even if we have no plans to host it elsewhere, the index will be more portable this way.

joeyparrish added a commit to joeyparrish/shaka-player that referenced this issue Apr 12, 2022
Rather than runtime-querying of appengine versions within the
appengine environment, we can instead generate the index at deployment
time (from git tags) and just serve static content.  This simplifies
the system and avoids dependence on Google Cloud.

This was less feasible before we adopted GitHub Actions, but is now
relatively simple.  The index will be regenerated when the index code
is updated or when a new release is created.

Closes shaka-project#4074
joeyparrish added a commit to joeyparrish/shaka-player that referenced this issue Apr 12, 2022
Rather than runtime-querying of appengine versions within the
appengine environment, we can instead generate the index at deployment
time (from git tags) and just serve static content.  This simplifies
the system and avoids dependence on Google Cloud.

This was less feasible before we adopted GitHub Actions, but is now
relatively simple.  The index will be regenerated when the index code
is updated or when a new release is created.

Closes shaka-project#4074
joeyparrish added a commit that referenced this issue Apr 12, 2022
Rather than runtime-querying of appengine versions within the
appengine environment, we can instead generate the index at deployment
time (from git tags) and just serve static content.  This simplifies
the system and avoids dependence on Google Cloud.

This was less feasible before we adopted GitHub Actions, but is now
relatively simple.  The index will be regenerated when the index code
is updated or when a new release is created.

Closes #4074
@joeyparrish joeyparrish reopened this Apr 12, 2022
@joeyparrish
Copy link
Member

Still broken, sadly. The automated deployment failed. But I have another fix, and I've tested it through a manual deployment this time.

joeyparrish added a commit to joeyparrish/shaka-player that referenced this issue Apr 12, 2022
Fixes these issues with the demo index deployment (tested by manual deployment):

 - Missing `handlers:` field in app.yaml
 - Missing handler for the URL `/`
 - Missing Flask entrypoint (even though no dynamic content is generated by it)

Closes shaka-project#4074
joeyparrish added a commit that referenced this issue Apr 12, 2022
Fixes these issues with the demo index deployment (tested by manual deployment):

 - Missing `handlers:` field in app.yaml
 - Missing handler for the URL `/`
 - Missing Flask entrypoint (even though no dynamic content is generated by it)

Closes #4074
@joeyparrish
Copy link
Member

Broken again! This time it seems to be caused by the workflow not having checked out the full repo history. Since the static content is based on git tags, we need those to be present. This didn't come up when I ran it locally, because of course, I had the full history in my local clone.

@joeyparrish joeyparrish reopened this Apr 13, 2022
@joeyparrish
Copy link
Member

While I'm waiting on review for PR #4126, I've done another manual push to fix the currently-deployed index. After the PR lands, the process should be fully automatic in the future.

joeyparrish added a commit that referenced this issue Apr 14, 2022
The demo index generation is now based on the git tags.  So when
checking out the repo to generate the index, the entire history is
needed.

Closes #4074
@joeyparrish joeyparrish reopened this Apr 14, 2022
@joeyparrish
Copy link
Member

Actions can be so obnoxious. There should be a linter for Actions YAML files, since a typo will ruin everything.

joeyparrish added a commit to joeyparrish/shaka-player that referenced this issue Apr 14, 2022
joeyparrish added a commit that referenced this issue Apr 15, 2022
joeyparrish pushed a commit that referenced this issue Apr 21, 2022
The script for deploying the demo version index page has a function
that chooses which versions to display. This script has a special
mode meant for testing that shows a default list of demo versions.
This mode was only supposed to trigger when run locally, but the
check did not work properly, so the demo version index was deploying
with a very out-of-date list of versions.
This fixes that check.

Closes #4074

<!--
Please remember to:

1. Use Conventional Commits syntax (fix: ..., feat: ..., etc.) in commits and
   PR title (https://www.conventionalcommits.org/)
2. Tag any related or fixed issues ("Issue #123", "Closes #420")
3. Sign the Google CLA if you haven't (https://cla.developers.google.com)

You may delete this comment from the PR description.
-->
@github-actions github-actions bot added the status: archived Archived and locked; will not be updated label Jun 14, 2022
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Jun 14, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.