-
Notifications
You must be signed in to change notification settings - Fork 34
Experimental package hub updates #4413
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
base: master
Are you sure you want to change the base?
Changes from 7 commits
7ca4500
fad9cdf
4757123
4bbd425
e855d87
16dbd51
640b525
20889d7
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -5,3 +5,4 @@ | |
| build/ | ||
| vendor/ | ||
| .vercel | ||
| node_modules/ | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,2 @@ | ||
| @dbt-labs:registry=https://npm.pkg.github.com | ||
| //npm.pkg.github.com/:_authToken=${GITHUB_TOKEN} | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. do we need the github token to be able to pull in
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I'm not sure on this, I think I have updated this now to use the builtin github actions for this. |
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,4 +1,6 @@ | ||
| require 'open-uri' | ||
| require 'json' | ||
| require 'time' | ||
|
|
||
| class Middleman::Util::EnhancedHash | ||
| # Mila: Looked into Hashie::Mash's source code where this class is | ||
|
|
@@ -151,6 +153,35 @@ def combine_packages(packages) | |
|
|
||
| set :package_index, combine_packages(@app.data.packages) | ||
|
|
||
| # Fetches repository metadata from the GitHub API. | ||
| # Returns an empty hash on any error so callers can treat missing fields as absent. | ||
| def fetch_github_repo(org, repo, token = nil) | ||
| 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' | ||
| } | ||
| options['Authorization'] = "Bearer #{token}" if token | ||
| JSON.parse(URI.open(url, options).read) | ||
| rescue => e | ||
| warn "[hub] GitHub API fetch failed for #{org}/#{repo}: #{e.message}" | ||
| {} | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 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!
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I've removed the 'rescue' block here. Claude suggested adding in timeouts so these have been added in as follows: |
||
| end | ||
|
|
||
| # 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'] | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 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?
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Thanks - I'll remove the github token now |
||
| featured_live = {} | ||
| @app.data.featured.each do |feat| | ||
| repo = fetch_github_repo(feat['org'], feat['package'], github_token) | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. does this slow down the build process significantly?
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 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) |
||
| live = { | ||
| 'description' => repo['description'], | ||
| }.compact | ||
| featured_live["#{feat['org']}/#{feat['package']}"] = live | ||
| end | ||
| set :featured_live, featured_live | ||
|
|
||
| after_configuration do | ||
|
|
||
| proxy "/api/v1/packages.json", | ||
|
|
@@ -220,6 +251,12 @@ def combine_packages(packages) | |
| ignore '/api/v1/package.template.json.erb' | ||
| ignore '/api/v1/raw.json.erb' | ||
|
|
||
| # Vendor CSS from npm (Biga tokens must load before Sourdough). Requires `npm install`. | ||
| import_file File.expand_path('node_modules/@dbt-labs/biga/tokens/tokens.css', root), | ||
| '/stylesheets/vendor/biga-tokens.css' | ||
| import_file File.expand_path('node_modules/@dbt-labs/sourdough/dist/sourdough.css', root), | ||
| '/stylesheets/vendor/sourdough.css' | ||
|
|
||
| activate :livereload | ||
|
|
||
| configure :development do | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,26 +1,44 @@ | ||
| [ | ||
| { | ||
| "org": "dbt-labs", | ||
| "package": "dbt_utils" | ||
| "package": "dbt_utils", | ||
| "tile": { | ||
| "description": "Utility functions for dbt projects" | ||
| } | ||
| }, | ||
| { | ||
| "org": "dbt-labs", | ||
| "package": "codegen" | ||
| "package": "codegen", | ||
| "tile": { | ||
| "description": "Macros that generate dbt code" | ||
| } | ||
| }, | ||
| { | ||
| "org": "dbt-labs", | ||
| "package": "dbt_project_evaluator" | ||
| "package": "dbt_project_evaluator", | ||
| "tile": { | ||
| "description": "This package contains macros and models to find DAG issues automatically" | ||
| } | ||
| }, | ||
| { | ||
| "org": "dbt-labs", | ||
| "package": "audit_helper" | ||
| "package": "audit_helper", | ||
| "tile": { | ||
| "description": "Useful macros when performing data audits" | ||
| } | ||
| }, | ||
| { | ||
| "org": "dbt-labs", | ||
| "package": "dbt_external_tables" | ||
| "package": "dbt_external_tables", | ||
| "tile": { | ||
| "description": "dbt macros to stage external source" | ||
| } | ||
| }, | ||
| { | ||
| "org": "metaplane", | ||
| "package": "dbt_expectations" | ||
| "package": "dbt_expectations", | ||
| "tile": { | ||
| "description": "Great Expectations tests for dbt models" | ||
| } | ||
| } | ||
| ] |
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.
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.
Uh oh!
There was an error while loading. Please reload this page.
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.
As above, I'll remove this now too