This project is mirrored from https://github.com/metabase/metabase.
Pull mirroring updated .
- Jun 13, 2022
-
-
Braden Shepherdson authored
Define identity-hash for fairly robust de-duplication when deserializing This is a fallback for fully robust de-duplication based on `entity_id` fields. All serialized models should support identity-hash, and there is a test to enforce that.
-
Ngoc Khuat authored
* add API to fetch and search card parameters * remove code to support linked filters * remove unecessary data in tests * fix from Noah's comments * some chain-filter -> param-values
-
- Jun 11, 2022
-
-
Ngoc Khuat authored
-
- Jun 10, 2022
-
-
Noah Moss authored
* allow str form as the first argument to deferred-tru * add tests and update doc strings * migrate more setting descriptions * Handle multiline translation sources ```clojure enumerate=> x (deferred-tru (str "Whether an introductory modal should be shown after the next database connection is added. " "Defaults to false if any non-default database has already finished syncing for this instance.")) enumerate=> (form->string-for-translation x) "Whether an introductory modal should be shown after the next database connection is added. Defaults to false if any non-default database has already finished syncing for this instance." ``` We get the form from `(g/grasp <file> ::translate)` which returns the `x`. And then simply pick through it for either a string literal or a call to `(str <literal>+)` Co-authored-by:
dan sutton <dan@dpsutton.com>
-
Howon Lee authored
Previously we defaulted to integer and float type in all cases for json unfolding type determination, even with bigints and bigdecimals. Now if we encounter a bignum in json unfolding we call it a bignum.
-
- Jun 09, 2022
-
-
dpsutton authored
* Correctly handle fall-through case. Condp is exhaustive and throws if nothing matches ``` mysql=> (condp = 2 1 :nope 3 :nope) Execution error (IllegalArgumentException) at metabase.driver.mysql/eval169387 (REPL:870). No matching clause: 2 mysql=> ``` and can take a single default clause at the end with no predicate ```clojure mysql=> (condp = 2 1 :nope :default-catch-all) :default-catch-all ``` This was attempted with a regex `#".*"` but this regex does not match everything! ```clojure mysql=> (driver/humanize-connection-error-message :mysql "hello\nthere") Execution error (IllegalArgumentException) at metabase.driver.mysql/eval167431$fn (mysql.clj:124). No matching clause: hello there mysql=> (re-matches #".*" "hi\nthere") nil ``` So just use use the message as the default to be returned which was the original intention. * Fix other databases as well
-
Noah Moss authored
* clear Slack cache last-updated value when resetting cache * use clear-channel-cache! function on invalid tokens as well
-
- Jun 08, 2022
-
-
Case Nelson authored
We want to make sure that the :persisted hydration is returned when updating a card's description.
-
dpsutton authored
* Save query executions for cached queries Previously had addressed https://github.com/metabase/metabase/pull/16720/ because we were including cache hit query executions in the running query execution time. IE, queries take 60 seconds to run but the cache hit takes 100ms. Our average thus would be (60s + 100ms)/2 ~ 30 seconds and the query duration falls below the cache threshold so we abandon the cache until the average creeps up back up again. Original issue: https://github.com/metabase/metabase/issues/15432 So the first fix was a poor one and it skipped recording the query execution at all which was way too heavy handed. ```clojure (when-not (:cached acc) (save-successful-query-execution! (:cached acc) execution-info @row-count)) ``` This approach takes the more considered approach: skip updating average execution time but record the query-execution. ```clojure ;; conditionally save average execution time (when-not (:cache_hit query-execution) (query/save-query-and-update-average-execution-time! query query-hash running-time)) ;; unconditionally (modulo having a context) save the query execution (if-not context (log/warn (trs "Cannot save QueryExecution, missing :context")) (db/insert! QueryExecution (dissoc query-execution :json_query))) ``` One question ------------ I'm unsure about one change here. What are the implications of not updating this execution time. There are two operations involved: ensuring there is a `Query` (table query) entry for a query and updating its average execution time. I'm assuming that since a query is cached it has a query table entry. But this code creates the Query if it doesn't exist and then updates its average execution time. It's not clear to me if we do in fact want to ensure a Query entry exists. But it feels harmless both in the case it is missing and also that it most likely isn't missing since we have a cached query and when it was cached this entry would have been created or updated. But adding a note just in case. * Fix test distinguish between the calls to save query execution and the calls to update average execution duration. The former should be called twice (for the uncached query and the cached query run) while the latter should only be called on the first run (aka, uncached results affect query duration stats, cached runs do not affect it) * Checksums are no more metadata became editable for models and the checksum really didn't add any value, just asserted that it had not been changed.
-
dpsutton authored
* Create po template file (pot) from clojure Rather than use xgettext we can use grasp to look for translation sites. The reason to do this is twofold: Multiline Translated Strings ---------------------------- We can use multiline strings in source to aide in readability. This [PR](https://github.com/metabase/metabase/pull/22901) was abandoned because we xgettext cannot be expected to combine string literals into a single string for translation purposes. But we can certainly do that with clojure code. ```clojure (defn- form->string-for-translation "Function that turns a form into the translation string. At the moment it is just the second arg of the form. Afterwards it will need to concat string literals in a `(str \"foo\" \"bar\")` situation. " [form] (second form)) (defn- analyze-translations [roots] (map (fn [result] (let [{:keys [line _col uri]} (meta result)] {:file (strip-roots uri) :line line :message (form->string-for-translation result)})) (g/grasp roots ::translate))) ``` `form` is the literal form. So we can easily grab all of the string literals out of it and join them here in our script. The seam is already written. Then reviving the PR linked earlier would upgrade the macros to understand that string literals OR `(str <literal>+)` are acceptable clauses. Translation context ------------------- Allowing for context in our strings. The po format allows for context in the file format. ``` msgctxt "The update is about changing a record, not a timestamp" msgid "Failed to notify {0} Database {1} updated" msgstr "" ``` See [this issue](https://github.com/metabase/metabase/issues/22871#issuecomment-1146947441) for an example situation. This wouldn't help in this particular instance because it is on the Frontend though. But we could have a format like ```clojure (trs "We" (comment "This is an abbreviation for Wednesday, not the possessive 'We'")) ``` The macro strips out the `comment` form and we can use it when building our pot file. Note there is a difficulty with this though since all source strings must be unique. There can be multiple locations for each translated string. ``` ,#: /metabase/models/field_values.clj:89 ,#: /metabase/models/params/chain_filter.clj:588 msgid "Field {0} does not exist." msgstr "" ``` The leading commas are present to prevent commit message comments. But if one location has a context and the other doesn't, or even worse, if they have two different contexts, we have a quandry: we can only express one context. Probably easy to solve or warn on, but a consideration. Caught Errors ------------- The script blew up on the following form: ```clojure (-> "Cycle detected resolving dependent visible-if properties for driver {0}: {1}" (trs driver cyclic-props)) ``` No tooling could (easily) handle this properly. Our macro assertions don't see the thread. But xgettext never found this translation literal. I warn in the pot generation so we can fix this. We could also have a test running in CI checking that all translations are strings and not symbols. Fundamental Tool ---------------- The sky is the limit because of this fundamental grasp tool: ```clojure enumerate=> (first (g/grasp single-file ::translate)) (trs "Failed to notify {0} Database {1} updated" driver id) enumerate=> (meta *1) {:line 35, :column 22, :uri "file:/Users/dan/projects/work/metabase/src/metabase/driver.clj"} ``` We can find all usages of tru/trs and friends and get their entire form and location. We can easily do whatever we want after that. Verifying Translation scripts still work ---------------------------------------- You can check a single file is valid with `msgcat <input.pot> -o combined.pot`. This will throw if the file is invalid. The general script still works: ``` ❯ ./update-translation-template [BABEL] Note: The code generator has deoptimised the styling of /Users/dan/projects/work/metabase/frontend/src/cljs/cljs.pprint.js as it exceeds the max of 500KB. [BABEL] Note: The code generator has deoptimised the styling of /Users/dan/projects/work/metabase/frontend/src/cljs/cljs.core.js as it exceeds the max of 500KB. [BABEL] Note: The code generator has deoptimised the styling of /Users/dan/projects/work/metabase/frontend/src/cljs/cljs-runtime/cljs.core.js as it exceeds the max of 500KB. Warning: environ value /Users/dan/.sdkman/candidates/java/current for key :java-home has been overwritten with /Users/dan/.sdkman/candidates/java/17.0.1-zulu/zulu-17.jdk/Contents/Home SLF4J: Failed to load class "org.slf4j.impl.StaticLoggerBinder". SLF4J: Defaulting to no-operation (NOP) logger implementation SLF4J: See http://www.slf4j.org/codes.html#StaticLoggerBinder for further details. Created pot file at ../../locales/metabase-backend.pot #<----- new line here Warning: environ value /Users/dan/.sdkman/candidates/java/current for key :java-home has been overwritten with /Users/dan/.sdkman/candidates/java/17.0.1-zulu/zulu-17.jdk/Contents/Home 2022-06-06 15:05:57,626 INFO metabase.util :: Maximum memory available to JVM: 8.0 GB Warning: protocol #'java-time.core/Amount is overwriting function abs WARNING: abs already refers to: #'clojure.core/abs in namespace: java-time.core, being replaced by: #'java-time.core/abs WARNING: abs already refers to: #'clojure.core/abs in namespace: java-time, being replaced by: #'java-time/abs 2022-06-06 15:06:01,368 WARN db.env :: WARNING: Using Metabase with an H2 application database is not recommended for production deployments. For production deployments, we highly recommend using Postgres, MySQL, or MariaDB instead. If you decide to continue to use H2, please be sure to back up the database file regularly. For more information, see https://metabase.com/docs/latest/operations-guide/migrating-from-h2.html 2022-06-06 15:06:03,594 INFO util.encryption :: Saved credentials encryption is DISABLED for this Metabase instance.
For more information, see https://metabase.com/docs/latest/operations-guide/encrypting-database-details-at-rest.html WARNING: abs already refers to: #'clojure.core/abs in namespace: taoensso.encore, being replaced by: #'taoensso.encore/abs WARNING: abs already refers to: #'clojure.core/abs in namespace: kixi.stats.math, being replaced by: #'kixi.stats.math/abs WARNING: abs already refers to: #'clojure.core/abs in namespace: kixi.stats.test, being replaced by: #'kixi.stats.math/abs WARNING: abs already refers to: #'clojure.core/abs in namespace: kixi.stats.distribution, being replaced by: #'kixi.stats.math/abs msgcat: msgid '{0} metric' is used without plural and with plural. msgcat: msgid '{0} table' is used without plural and with plural. ``` I'm not sure what the last two lines are about but I suspect they are preexisting conditions. their form from the final combined pot file (althrough again with leading commas on the filenames to prevent them from being omitted from the commit message) ``` ,#: frontend/src/metabase/admin/permissions/components/PermissionsConfirm.jsx:32 ,#: src/metabase/automagic_dashboards/core.clj ,#: target/classes/metabase/automagic_dashboards/core.clj ,#, fuzzy, javascript-format msgid "{0} table" msgid_plural "{0} tables" msgstr[0] "" "#-#-#-#-# metabase-frontend.pot #-#-#-#-#\n" "#-#-#-#-# metabase-backend.pot (metabase) #-#-#-#-#\n" msgstr[1] "#-#-#-#-# metabase-frontend.pot #-#-#-#-#\n" ... ,#: frontend/src/metabase/query_builder/components/view/QuestionDescription.jsx:24 ,#: src/metabase/automagic_dashboards/core.clj ,#: target/classes/metabase/automagic_dashboards/core.clj ,#, fuzzy, javascript-format msgid "{0} metric" msgid_plural "{0} metrics" msgstr[0] "" "#-#-#-#-# metabase-frontend.pot #-#-#-#-#\n" "#-#-#-#-# metabase-backend.pot (metabase) #-#-#-#-#\n" msgstr[1] "#-#-#-#-# metabase-frontend.pot #-#-#-#-#\n" ``` * Add drivers, one override, remove unused import import wasn't necessary forgot to check the driver sources for i18n for some reason grasp doesn't descend into ```clojure (defmacro ^:private deffingerprinter [field-type transducer] {:pre [(keyword? field-type)]} (let [field-type [field-type :Semantic/* :Relation/*]] `(defmethod fingerprinter ~field-type [field#] (with-error-handling (with-global-fingerprinter (redux/post-complete ~transducer (fn [fingerprint#] {:type {~(first field-type) fingerprint#}}))) (trs "Error generating fingerprint for {0}" (sync-util/name-for-logging field#)))))) ``` I've opened an issue on [grasp](https://github.com/borkdude/grasp/issues/28) * Use vars rather than name based matching -
Ngoc Khuat authored
* add :updated-at-timestamped? property and use it on some models
-
dpsutton authored
-
metamben authored
We have already had support for server authentication based on custom certificates. This change adds support for authenticating the client based on custom client key and certificate.
-
- Jun 07, 2022
-
-
Case Nelson authored
* Include :features whether or not db is initialized On restart, drivers may not be initialized but admins still need to know what features are available. Always include features on db post-select. * Driver can be null when not selected * Remove intitialized check for :details as well Both driver.u/features and driver/normalize-db-details eventually call `dispatch-on-initialized-driver` which will call `initialize-driver-if-needed`, removing the need for the check. * Unregistered drivers can make it in here, but they can be abstract so driver/available is not apropriate
-
Howon Lee authored
Do trivial identity for passthrough of symbols for nested field column instead of default processing (#23136) Should whack both #23026 and #23027. Previous (default) behavior of the reducible query that gets the limited contents of the JSON to break out the nested field columns is to lowercase identifiers. This is root cause of #23026 and #23027. But we want the proper case of those identifiers in order to be modifying the query correctly when we query the JSON. So we set the reducible query to just pass through the identifiers instead of default behavior.
-
Luiz Arakaki authored
-
- Jun 06, 2022
-
-
metamben authored
Fixes #22626.
-
Noah Moss authored
-
Case Nelson authored
* Handle deletable state when enabling persistence When enabling persistence all models are supposed to be set to creating unless they have been explicitly turned "off". Before, we were only looking for new models without attached PersistedInfos when enabling, but missed deletable PersistedInfos that needed to be re-enabled. It's the correct thing to do during the refresh as you wouldn't want refresh to re-enable but it's incorrect when enabling. * Remove unecessary thread-last
-
- Jun 03, 2022
-
-
Gustavo Saiani authored
-
Ngoc Khuat authored
* dashboard api to get params value should return 404 instead of 500 when param not found * on the 2nd thought, let's return 400 * update the tests too * wording
-
Ngoc Khuat authored
* add parameters to report_card and card APIs * make sure migration can work with existing cards and add migration tests * remove debug code * When parameters is empty, return template tag parameters * update schema message * update migraiton test name * minor changes in test * fix failing tests * Always use card.parameters in PublicQuestion * Add defaults and readd isEmpty check * Default to undefined/use template tags in PublicQuestion... * parameters should be in the writableProperties * Native query card: construct parameters from template tags * Separate the generation of parameter id * Add parameter_mappings to report_card (#23003) * add parameter_mappings to report_card * fix from Noah's comments * fix from Noah's comments * Update a parameter from an updated template tag * Correct the parameters construction * Also add `parameter_mappings` to writableProperties * CI test: bust the npm module cache * Revert "CI test: bust the npm module cache" This reverts commit 5a327b616f0220f43a90f7f871e0bd877ffa6f47. Co-authored-by:
Dalton Johnson <daltojohnso@users.noreply.github.com> Co-authored-by:
Ariya Hidayat <ariya@metabase.com>
-
Howon Lee authored
Adds option to turn JSON column unfolding off. This does not constitute an actual fix of boat161 but flamber wanted it
-
- Jun 02, 2022
-
-
Gustavo Saiani authored
-
- Jun 01, 2022
-
-
Braden Shepherdson authored
-
- May 31, 2022
-
-
Braden Shepherdson authored
That is: collection, dimension, metric, native_query_snippet, pulse, report_card, report_dashboard, report_dashcard, segment, timeline Notably that doesn't include database, table, or field, since those all have external unique IDs that are used instead.
-
Case Nelson authored
* Include field annotations for native queries too Persistence will replace a source-table source-query with a native query, but preprocess has still filled in source-metadata with all of the relevant field-ids expected to be returned. With this change we include field info from the store in the same way that mbql-cols does. This allows persisted models to honor field settings like `:visibility :details-only`. * Force type of merge-source-metadata-col to map By doing the lookup to store/field at the top of the merge, the type of annotations coming through was a FieldInstance. Tests, at least, were unhappy about this and it's better not to change it. * Resolve fields for ids in source-metadata Makes sure that the qp/store has all the available fields for annotations. * Recursively find source-metadata field-ids for annotations * Use transducer as per review
-
dpsutton authored
Addresses part of https://github.com/metabase/metabase/issues/8679 Pivot tables can have subqueries that run to create tallies. We do not hold the entirety of resultsets in memory so we have a bit of an inversion of control flow: connections are opened, queries run, and result sets are transduced and then the connection closed. The error here was that subsequent queries for the pivot were run while the first connection is still held open. But the connection is no longer needed. But enough pivots running at the same time in a dashboard can create a deadlock where the subqueries need a new connection, but the main queries cannot be released until the subqueries have completed. Also, rf management is critical. It's completion arity must be called once and only once. We also have middleware that need to be composed (format, etc) and others that can only be composed once (limit). We have to save the original reducing function before composition (this is the one that can write to the download writer, etc) but compose it each time we use it with `(rff metadata)` so we have the format and other middleware. Keeping this distinction in mind will save you lots of time. (The limit query will ignore all subsequent rows if you just grab the output of `(rff metadata)` and not the rf returned from the `:rff` key on the context. But this takes the following connection management: ``` tap> "OPENING CONNECTION 0" tap> "already open: " tap> "OPENING CONNECTION 1" tap> "already open: 0" tap> "CLOSING CONNECTION 1" tap> "OPENING CONNECTION 2" tap> "already open: 0" tap> "CLOSING CONNECTION 2" tap> "OPENING CONNECTION 3" tap> "already open: 0" tap> "CLOSING CONNECTION 3" tap> "CLOSING CONNECTION 0" ``` and properly sequences it so that connection 0 is closed before opening connection 1. It hijacks the executef to just pass that function into the reducef part so we can reduce multiple times and therefore control the connections. Otherwise the reducef happens "inside" of the executef at which point the connection is closed. Care is taken to ensure that: - the init is only called once (subsequent queries have the init of the rf overridden to just return `init` (the acc passed in) rather than `(rf)` - the completion arity is only called once (use of `(completing rf)` and the reducing function in the subsequent queries is just `([acc] acc)` and does not call `(rf acc)`. Remember this is just on the lower reducing function and all of the takes, formats, etc _above_ it will have the completion arity called because we are using transduce. The completion arity is what takes the volatile rows and row counts and actually nests them in the `{:data {:rows []}` structure. Without calling that once (and ONLY once) you end up with no actual results. they are just in memory.
-
- May 30, 2022
-
-
Ngoc Khuat authored
remove param_values hydration keys
-
- May 27, 2022
-
-
Howon Lee authored
Limit the length of the row for JSON parsing. This was the simplest way to make another attack at #22636.
-
- May 26, 2022
-
-
Noah Moss authored
-
- May 25, 2022
-
-
Case Nelson authored
* Add can-manage to databases to help front-end * Only return persisted true when the state agrees * Add event for persisted-info to catch new models and turn on peristence when appropriate * Fix bad threading * Add comment about the case being detected to add the persisted-info * Fix pre-existing bug in expand-db-details where a function was used as the condition
-
metamben authored
* Indicate field responsible for database error * Implement new interface * Make error showing logic a little easier to understand * Show individual field error in database form * Add backend tests for connection errors when adding a database * Add new form error component * Make database form error message pretty * Make main database form error message field bold * Change create account database form according to feedback * Fix failed E2E tests from UI changed. * Make it easier to tree-shake "lodash" * Change according to PR review * Cleanup + remove FormError (to be included in a separate PR) Co-authored-by:
Mahatthana Nomsawadi <mahatthana.n@gmail.com>
-
Anton Kulyk authored
* Expose `persisted-models-enabled` to authenticated * Remove `isAdmin` check for showing peristence toggle
-
- May 24, 2022
-
-
Case Nelson authored
Since we have linters that force the refers to be both sorted and max lined, adding new models in the middled forced people to realign the whole refer vector. Having run into this twice, if we keep refers each on their own line the initial add, as well as merges will both be much less painful.
-
Case Nelson authored
* Add anchor time to persistence schedule Introduces a new public-setting persisted-model-refresh-anchor-time, representing the time to begin the refresh job. Defaults to midnight "00:00". Also peg the cron jobs to run in the first of: reporting timezone, system timezone, or UTC. This means that if a user needs the refresh to be anchored consistently to a certain time in a certain place, they need to update reporting timezone to match. * Add tests for anchor time * Force anchor-time to midnight if refresh hours is less than 6 * Need to check that hours was passed in * Fix ns lint * Add tests to ensure timezone is being set on trigger
-
Noah Moss authored
-
- May 23, 2022
-
-
dpsutton authored
we assert that the schedule gets randomized and it is no longer one of the default schedules. We need to make sure that it cannot return one of the default schedules in that case :). Low probability (1 in 58) but with 28 test suites run for each commit it starts to add up and becomes Flaky™
-
dpsutton authored
Settings require db access so cannot be called at read/require time. Not until we have initialized the db are setting values possible to be read.
-
dpsutton authored
* Handle `nil` cache during initial cache population this bug only happens soon after startup. The general idea of the cache is that we repopulate if needed, and other threads can use the stale version while repopulating. This is valid except at startup before the cache has finished populating. The `(setting.cache/cache)` value will still be nil to other threads while the first thread populates it. this is a race but sounds like an unlikely one to hit in practice. But we have a special case: `site-url`. We have middleware that checks the site-url on every api request and sets it if it is nil. So if an instance gets two requests before the cache has been populated, it will set it to whatever value is in the headers, overriding whatever has been set in the UI. The following snippet was how i diagnosed this. But to simulate startup you can just `(require 'metabase.models.setting.cache :reload)` to get back to an "empty" cache and let the threads race each other. I also put high contention on reloading by dropping the millisecond cache update interval to 7 milliseconds. But this refresh interval does not matter: we just fall back to the old version of the cache. It is only the initial cache population that is using `nil` as a current cache. ```clojure public-settings=> (let [mismatch1 (atom []) mismatch2 (atom []) iterations 100000 latch (java.util.concurrent.CountDownLatch. 2) nil-value (atom [])] (future (dotimes [_ iterations] (let [value (site-url) cache (metabase.models.setting.cache/cache)] (when (not= value "http://localhost:3000") (swap! mismatch1 conj value)) (when (not= (get cache "site-url") "http://localhost:3000") (swap! nil-value conj cache)))) (.countDown latch)) (future (dotimes [_ iterations] (let [value (site-url) cache (metabase.models.setting.cache/cache)] (when (not= value "http://localhost:3000") (swap! mismatch2 conj value)) (when (not= (get cache "site-url") "http://localhost:3000") (swap! nil-value conj cache)))) (.countDown latch)) (.await latch) (def nil-value nil-value) [(count @mismatch1) (take 10 @mismatch1) (count @mismatch2) (take 10 @mismatch2)]) [0 () 1616 (nil nil nil nil nil nil nil nil nil nil)] ``` * Don't attempt to get setting values from db/cache before db ready * We check `db-is-setup?` above this level db check has to happen higher up at `db-or-cache-value` as neither the db will work, nor the cache based on selecting from the db, if the db is not set up. * Remove some of the nested whens (thanks howon) Lots of nesting due to requiring and checking for nil of the required var. This can never really be nil but just to overly cautiously guard, put in an or with the `(constantly false)`. Combine the two predicates into a single `and` and then swap our homegrown `(when (seq x) x)` for the equivalent `(not-empty x)`.
-