fix: config loading #1955
Merged
fix: config loading #1955
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Deploying with
|
| Latest commit: |
53f43a1
|
| Status: | ✅ Deploy successful! |
| Preview URL: | https://2f1b4339.nft-storage-1at.pages.dev |
yusefnapora
approved these changes
Jun 2, 2022
There was a problem hiding this comment.
Very nice, thanks @hugomrdias. I like that you moved the default config values out of the code and into the template env. In hindsight its simpler to have just the one source of truth.
|
@hugomrdias I'll go ahead and merge, since this was blocking production deploys. |
|
Awesome thanks @hugomrdias and @yusefnapora . Deploys are now working 🎉 |
gobengo
pushed a commit
to gobengo/nft.storage
that referenced
this pull request
Jun 14, 2022
gobengo
added a commit
that referenced
this pull request
Jun 16, 2022
* add .github/workflows/cron-nft-ttr * nft-ttr uses yargs to accept --url flag defaulting to nft.storage * nft-ttr downloads file to use as example image when measuring * nft-ttr downloads a source file to use for rest of test * measureNftTimeToRetrievability.js logs duration * measureNftTimeToRetrievability.js pushes metrics to metricsGateway * measureNftTimeToRetrievability.spec.js test doesnt hit network, asserts log of retrieval duration * Add github workflow to typecheck/test cron * lint cron * cron typechecks * github workflow for cron check does npm-install in cron dir * try new workflow cron check install-command * dont use gozala typecheck action * gh cron log * cron debug * client has prepack script * cron debug * cron debug * cron rm mime-types * cron debug * cron debug * measureNftTimeToRetrievability can be tested with >10Mb files * move online test to nft-ttr.spec.js * update cron-nft-ttr.yml * nft-ttr support retrieving from multiple gateways * fix cron typecheck * .github cron-nft-ttr invokes using multiple gateways * github cron-nft-ttr passes secrets.PUSHGATEWAY_BASIC_AUTH * measureNftTimeToRetrievability.js metrics pushgateway works locally with sample value from secrets vault * rm default metricsPushGateway and provide it in gh action * yarn.lock * ghwf cron-nft-ttr invokes with camelCase flag not kebab-case * ci(cron): action doesnt explicitly check for NFT_STORAGE_API_KEY env * rename BYTES_10_MB -> defaultTestMinImageSizeBytes address #1945 (comment) * add descriptive variable name isProcessEntrypoint in bin/nft-ttr.js * rm unused UrlIamages * rename fn TestImages -> createTestImages * createTestImages uses RandomImageBlob * packages/cron uses @typescript-eslint/recommended and naming-convention * measureNftTimeToRetrievability.js merges config & { secrets } into options * add Milliseconds.subtract static * measureNftTimeToRetrievability checks success via response.ok and not status allowList addresses https://github.com/nftstorage/nft.storage/pull/1945/files/efa794b23197f36ecad68c93f377e7a2e59dc9d3#diff-70187cabafb845fa5768abccc9ef51efd84529fb89e9f37188058f90fde3d69d * cron eslintConfig rm unneeded .excludedFiles * dont eslint-disable cron jobs/dagcargo , disable via eslintConfig overrides instead * start integrating prom-client * refactor metrics * progress on metrics * measureNftTimeToRetrievability test works on airplane with no wifi * nft-ttr uses httpImageFetcher * measureNftTimeToRetrievability.spec asserts that pushRetrievalMetrics is called * cleanup cron nftttr * cron uses sade instead of yargs * cron tests work offline by stubbing network calls * cron package uses ava instead of jest for testing * measureNftTimeToRetrievability and nft-ttr return AsyncIterable and tests assert that instead of recorded info logs * cron eslint uses plugin:prettier/recommended * cron eslint add plugin:@typescript-eslint/recommended-requiring-type-checking * ci: cron.yml prepares client before linting cron * remove unused code pushing to prom pushgateway via fetch (and using wrong metric type) * remove unused byteLength field from createRetrievalDurationSecondsMetric * cron test uses c8 to report code coverage * cron remove unused safe-env-vars dep * cron remove unused dep debug * cron eslint remove unused not-from-recommended rules * cron eslint remove unused eslintIgnore values * cron remove unused eslint-disable file comments * clean createMeasureSecretsFromEnv based on review * nft-ttr error throws with cause instead of console.warn Co-authored-by: Alan Shaw <alan.shaw@protocol.ai> * fix: preserve all backup_urls for chunked uploads (#1940) * fix: preserve all backup_urls for chunked uploads * rm stray file * respond to review feedback * chore: refactor service configuration loading (#1914) * chore: refactor service configuration loading Refactors the way we load configuration variables. Key changes: - constants.js is replaced with config.js - config.js exports `getServiceConfiguration`, which lazily loads the config into a JS `ServiceConfiguration` object - default values for unit tests are defined in code, in config.js - runtime values are dynamically looked up on `globalThis`, to avoid undefined reference errors if a var is missing. - if a config variable doesn't exist at runtime, we log a warning and use the default value All existing tests pass, but the config code itself isn't tested yet. * embrace truthiness * add test for serviceConfigFromVariables * fancier bool parsing * add tests for loading config vars & throw on missing vars in prod/staging * add hack to workaround global VERSION issue * include all missing vars in error / warning message * flatten out config object & simplify default value fallback * fixup for prev commit: get maintenance mode from config * chore: add namespace to VERSION, etc * fix maintenence mode tests * rm dev cruft * fix rebase cruft * feat: Adding HasDeleteRestriction user_tag (#1952) * Adding the type and failing HTTP DELETE operations if this tag is set. * See nftstorage/admin.storage#66 * chore(main): release api 2.24.2 (#1930) Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com> * feat: DB schema and API for user_tag_proposal. (#1410) (#1402) * Users create records in this table and admins manage it. Co-authored-by: trigramdev9 <jsdevel@trigram.co> * fix: config loading required vars (#1955) * feat: Adding admin ability to search by github_id (#1960) * See nftstorage/admin.storage#68 * chore(main): release api 2.25.0 (#1956) Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com> * feat: optimize metrics query (#1906) This commit optimizes the metrics query to be less brittle in the case that the dimensions array data changes or takes on new values. Here, we query the presence of the keys instead of the exact match of the stringified array. * chore(main): release api 2.26.0 (#1966) Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com> * feat: cron release workflow (#1932) * chore(main): release cron 3.1.0 (#1968) Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com> * feat: add NFTUp CTA to stats page (#1971) * chore(main): release website 1.52.0 (#1975) Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com> * fix: Adding user_tag_proposal schema to reset.sql for local dev (#1977) * Introduced in #1402 * docs: add store from URL example (#1979) Adds an example showing how to store ERC-721 compatible metadata with image data retrieved from a URL on the internet. Adapted from https://github.com/nftstorage/nft.storage/blob/main/examples/client/browser/store.html * Revert "nft-ttr error throws with cause instead of console.warn" This reverts commit 726a7a4. * nft-ttr rename main -> cli * nft-ttr tests pass without process.env needing NFT_STORAGE_API_KEY * cron: better typechecking around RetrievalDurationMetric value * test createMeasureOptionsFromSade * test createMeasureSecretsFromEnv * rm sadeParseWithoutExit * get rid of LogFunction * test cron utils so coverage is green in c8 * rm --silent from cron-nft-ttr gh wf * remove unneeded eslint-disable-next-line * remove commented console.log * Add explanation comment to hasOwnProperty * rm unnecessary nohoists Co-authored-by: Alan Shaw <alan.shaw@protocol.ai> Co-authored-by: Yusef Napora <yusef@protocol.ai> Co-authored-by: Joe Spencer <jsdevel@trigram.co> Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com> Co-authored-by: Hugo Dias <hugomrdias@gmail.com> Co-authored-by: Johnathon Roach <johnathonroach@users.noreply.github.com> Co-authored-by: Paolo Chillari <flea89@users.noreply.github.com>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Should fix config loading issue reported here #1914 (comment)