Experimental package hub updates - #4413
Conversation
Routes @dbt-labs packages to GitHub Packages registry. Requires GITHUB_TOKEN to be set in the local environment. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
joellabes
left a comment
There was a problem hiding this comment.
LGTM, but I'm not 100% sold on including the last package release date on the tiles, as the featured packages are reasonably stable and a stale publish date does not imply anything bad
Also, node_modules shouldn't be committed should it?
cmcarthur
left a comment
There was a problem hiding this comment.
please remove node_modules from the commit, I will take another pass after that! thanks @ellenfogarty-dbt
Add node_modules/ to .gitignore and untrack all committed node_modules files. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Updated to remove package date from featured package tiles and node_modules from the commit.
cmcarthur
left a comment
There was a problem hiding this comment.
@ellenfogarty-dbt thanks for removing node_modules. I have some more feedback, can you take a look? thanks
|
|
||
| # Fetch live description for each featured package at build time. | ||
| # Falls back gracefully to the values in data/featured.json if the API is unavailable. | ||
| github_token = ENV['GITHUB_TOKEN'] |
There was a problem hiding this comment.
this won't work in CI/CD, we don't make a github token available to the build process. shouldn't these be public repositories anyway? why do we need a github token?
There was a problem hiding this comment.
Thanks - I'll remove the github token now
| AWS_REGION: us-east-1 | ||
| AWS_ACCESS_KEY_ID: ${{ secrets.AWS_ACCESS_KEY_ID }} | ||
| AWS_SECRET_ACCESS_KEY: ${{ secrets.AWS_SECRET_ACCESS_KEY }} | ||
| GITHUB_TOKEN: ${{ secrets.GITHUB_TOKEN }} |
There was a problem hiding this comment.
this is not set, and I don't think we should set it. if we need github auth, we can use the builtin github actions permission model, but I don't think it's necessary here, and it wouldn't have any special permissions to the repositories that host packages.
There was a problem hiding this comment.
As above, I'll remove this now too
| @@ -0,0 +1,2 @@ | |||
| @dbt-labs:registry=https://npm.pkg.github.com | |||
| //npm.pkg.github.com/:_authToken=${GITHUB_TOKEN} | |||
There was a problem hiding this comment.
do we need the github token to be able to pull in biga as well?
There was a problem hiding this comment.
I'm not sure on this, I think I have updated this now to use the builtin github actions for this.
| github_token = ENV['GITHUB_TOKEN'] | ||
| featured_live = {} | ||
| @app.data.featured.each do |feat| | ||
| repo = fetch_github_repo(feat['org'], feat['package'], github_token) |
There was a problem hiding this comment.
does this slow down the build process significantly?
There was a problem hiding this comment.
TBH I'm not sure on this one. My knowledge is pretty limited on this. I've ran it past Claude and gotten the following response - keen to know your thoughts:
The more meaningful risk is reliability, not speed — if GitHub's API is slow or down, each failed request waits for a timeout before moving on. The rescue block in fetch_github_repo does catch errors and fall back gracefully, but depending on what timeout URI.open applies by default (Ruby doesn't set one), a hung connection could stall the build noticeably.
You could add an explicit read timeout to make the fallback snappy:
def fetch_github_repo(org, repo)
url = "https://api.github.com/repos/#{org}/#{repo}"
options = {
'Accept' => 'application/vnd.github+json',
'X-GitHub-Api-Version' => '2022-11-28',
'User-Agent' => 'hub.getdbt.com-middleman-build',
open_timeout: 5,
read_timeout: 5
}
JSON.parse(URI.open(url, options).read)
rescue => e
warn "[hub] GitHub API fetch failed for #{org}/#{repo}: #{e.message}"
{}
end
| JSON.parse(URI.open(url, options).read) | ||
| rescue => e | ||
| warn "[hub] GitHub API fetch failed for #{org}/#{repo}: #{e.message}" | ||
| {} |
There was a problem hiding this comment.
I don't like the idea that the site will render differently based on whether the github api was up or down during the time that we built it. IMO this should fail hard!
There was a problem hiding this comment.
I've removed the 'rescue' block here. Claude suggested adding in timeouts so these have been added in as follows:
open_timeout: 5,
read_timeout: 5
Updates as per PR comments
Following on from Fusion compatibility badge updates, these are some experimental UI updates in package hub to: