This project is mirrored from https://github.com/metabase/metabase.
Pull mirroring updated .
- Aug 05, 2022
-
-
Noah Moss authored
* Bump shadow-cljs This lets us use the template tag functionality ``` add experimental support for creating js template strings used like str but emits native JS `` template (js-template "foo" (+ 1 2) "bar") emits the literal `foo${(1 + 2)}bar` ``` But this required a few changes on our side as well. Required for this: -- LOGGING Bumped glogi ```diff ;; new stuff - [lambdaisland/glogi "1.0.106"]] - + [com.lambdaisland/glogi "1.1.144"]] ``` Annoying because it had a group name change due to Clojars policy. This had the same error as us below. That's why I'm hopeful that it will actually just work. There were some changes with the Google Closure library with things not included or referenceable. So I just removed these imports and the typehints that used them. We'll need to verify that this still works but both compiling for dev and release (with advanced compilation) work. Still possible there will be some runtime...
-
Reza Lotun authored
-
Reza Lotun authored
-
dpsutton authored
* Fix in-memory logger Our Admin > Troubleshooting > Logs page broke, just showing a spinner and never showing the logs. Don't quite understand why this fixes it. Javadocs are https://logging.apache.org/log4j/2.x/log4j-api/apidocs/org/apache/logging/log4j/LogManager.html#getContext-boolean- ```clojure logger=> (log/warn "test") nil logger=> (count @messages*) 0 ;; no in-memory logs so page is empty ;; change `(LogManager/getContext true)` in the momoized ns-logger fn ;; and then observe: logger=> (log/warn "test") nil logger=> (count @messages*) 4 ``` Some explorations that might shine some light: ```clojure logger=> (into {} (.getAppenders (.getLogger (LogManager/getContext false) (str *ns*)))) {} logger=> (into {} (.getAppenders (.getLogger (LogManager/getContext true) (str *ns*)))) {"metabase-appender" #object[metabase.logger.proxy$org.apache.logging.log4j.core.appender.AbstractAppender$ff19274a "0x4d680247" "metabase-appender"]} ``` So something is not hooked up quite right. * Add tests for metabase.logger * Ensure `api/util/logs` returns logs * tests * Check for presence of `MetabaseLoggerFactory` rather than whole str When in a namespace with a record, `Foo resolves to ns.Foo. But outside it resolves to ns/Foo. When running tests from the command line *ns* is user so it gets more complicated. * Kinda playing whackamole™ with `(LogManager/getContext true)` * Remove custom memoizing logger History: 39.2 we set `Multi-Release: true` in our manifest file and query speed drops like a stone. Jeff was able to track this to our logger calls in tight loops. We revert the multi-release and keep seeing the warning on startup > WARNING: sun.reflect.Reflection.getCallerClass is not supported. This will impact performance. Benchmarking on 39.2 (bench (log/trace "hi")) -> 15383 ns So we freaked out and set up a memoizing logger factory (bench (log/trace "hi")) -> 141 ns What a win. But we never noticed that the default *logger-factory* being picked up was slf4j ( `(log.impl/name log/*logger-factory*)` -> "org.slf4j" ). On 39.2 if you set the factory to the log4j2 version you get back to a great number: `(bench (log/trace "hi"))` -> 25 ns And thus ensuring that our logger is the default log4j2 version is even faster than our memoizing logger-factory. Memoizing factory: 141 ns slf4j factory: 2269 ns log4j2 factory: 31 ns What does `(LogManager/getContext false)` mean versus using `true`? We only need and want a single context. But log4j2 by default uses a context selector called `ClassLoaderContextSelector`. We could put all of this behind us if we used a context selector type `BasicContextSelector` but this is surprisingly hard to do: you have to set a system property. And since all of this stuff gets initialized in static initializers, we don't have an opportunity to do this programmatically. The way around this is to require people to pass this system property on startup which is not acceptable. So getContext true checks for a threadlocal context in a specific static variable and falls back to a Default context. getContext false looks at classloaders and ends up at a different context. BUT: the log.impl version uses a closure over getContext false instead of getting it each time. And I suspect that when it does this there is only one so it is the default and continues to use this one. In our LoggerFactory implementation we were looking up the context each time. This still seems to work and everything is playing nice in our application classloader but its totally possible that our drivers are not hitting this. I'll have to investigate this. Verification: - build the uberjar locally (`bin/build`) - copy to some temp directory and also copy criterium.jar ```shell MB_JETTY_PORT=4000 java "$(socket-repl 4001)" -cp locally-built.jar:criterium.jar metabase.core ``` ```clojure /tmp/locally-built via
v17.30 on metabase-query ❯ nc localhost 4001 user=> (doto 'metabase.logger require in-ns) metabase.logger metabase.logger=> (require '[criterium.core :refer [bench]]) nil metabase.logger=> (bench (log/trace "hi")) Evaluation count : 1686535500 in 60 samples of 28108925 calls. Execution time mean : 22.487972 ns Execution time std-deviation : 0.101004 ns Execution time lower quantile : 22.326806 ns ( 2.5%) Execution time upper quantile : 22.648368 ns (97.5%) Overhead used : 6.924761 ns nil metabase.logger=> (count (messages)) 358 metabase.logger=> ``` Verifies that we are on the order of 22 nanoseconds and the in-memory logger has messages in it. * Appease our namespace linters * I'll unplug you ns linter * Better tests and ns docstring * Bootstrap to set system properties New entrypoint for the application: metabase.bootstrap sets two properties for logging (context selector, log4j2 factory) and ensures those properties are set before any logging code is loaded. * docstrings and clean ns * metabase.logger ns docstring cleanup * docstring * rename a test now that there's no memoization * add logger properties to :dev profile * Revert "add logger properties to :dev profile" This reverts commit 4f09fa3b631f882a3c5edcab4508769ffb20d4fa. * deps -
Nick Fitzpatrick authored
-
Aleksandr Lesnenko authored
-
Jeff Bruemmer authored
-
Natalie authored
-
Gustavo Saiani authored
-
Diogo Mendes authored
-
- Aug 04, 2022
-
-
Natalie authored
-
metamben authored
Breaking date values into parts should happen in the time zone expected by the user and the construction of the truncated date should happen in the same timezone. This is especially important when the difference between the expected timezone and UTC is not an integer number of hours and the truncation resolution is hour. (See #11149).
-
Nick Fitzpatrick authored
-
metamben authored
-
metamben authored
-
Ryan Laurie authored
* Dashboard Card String Filters are now case-insensitive This PR is a draft because while it solves the problem of string filters being case sensitive, it doesn't necessarily do it in the best way. This isn't necessarily a bug, but it seems that there is no way for the frontend to set the :case-sensitive true/false option anyway. For the purposes of this initial attempt at a solution, I have modified the endpoint ` POST "/:dashboard-id/dashcard/:dashcard-id/card/:card-id/query"` to automatically include an option map containing :case-sensitive false. The machinery to take this option into consideration already exists with the default :sql ->honeysql function in the ` metabase.driver.sql.query-processor` namespace. See the `like-clause` private function in particular. Since the query processor is a bit opaque to me at present, I was unable to figure out if there is a proper way that the frontend could send an options map (or key-value pair) all the way through the qp middleware to the query building stage. I discovered that if you conj a map `{:case-sensitive false}` to the output of the `to-clause` function in `metabase.driver.common.parameters.operators`, you will get the desired case-insensitive behavior. So, I modified the to-clause function in this PR to appropriately conj an options map if one exists. What I'd like to know: - is there a super-simple way to pass an option in already that I just missed? (eg. I thought perhaps in the `[:field 13 nil]` that 'nil' could be an options map, but I couldn't get that to work for me) - is there a middleware approach that I should consider? - any other options to appropriately hanlde this? * Revert the endpoint. If the frontend sends an options map on an options key inside the parameter, this endpoint will pass that on, so no change is needed. * include parameter options in datasetQuery Co-authored-by:
Adam James <adam.vermeer2@gmail.com>
-
Cam Saul authored
* Fix SSH tunnel leaks when testing connections * Appease kondo =( * Add some more NOTES
-
Braden Shepherdson authored
These fields hold JSON, which can contain field IDs in a few places. Particularly nasty is the `:column_settings` subfield, which is a map whose *keys* are JSON strings with field IDs.
-
metamben authored
See https://github.com/metabase/metabase/issues/22867 for context.
-
Natalie authored
-
Jeff Bruemmer authored
-
Alexander Polyankin authored
-
Aleksandr Lesnenko authored
-
dpsutton authored
If you click "view sql" on a cached model you see the substituted query rather than the "real" query. The query based on cache: ```sql SELECT "source"."login" AS "login", "source"."count" AS "count" FROM (select "login", "count" from "metabase_cache_e449f_19"."model_4112_issue_assi") "source" LIMIT 1048575 ``` The real underlying query: ```sql SELECT "source"."login" AS "login", "source"."count" AS "count" FROM (SELECT "github_raw"."issue_events__issue__assignees"."login" AS "login", count(*) AS "count" FROM "github_raw"."issue_events__issue__assignees" GROUP BY "github_raw"."issue_events__issue__assignees"."login" ORDER BY "count" DESC, "github_raw"."issue_events__issue__assignees"."login" ASC) "source" LIMIT 1048575 ``` If you click the "convert to sql question" you will end up with a sql question that is not based on the original question but the Metabase managed table.
-
Ngoc Khuat authored
-
Alexander Polyankin authored
-
Dalton authored
-
Dalton authored
-
Dalton authored
* Update hasPermissionsToMap to check for existence of dataset_query * Add check to mapping-options fn, too * Add repro for #24536 * Uncomment and enable the repro Co-authored-by:
Nemanja <31325167+nemanjaglumac@users.noreply.github.com>
-
Alexander Polyankin authored
-
- Aug 03, 2022
-
-
Natalie authored
-
Noah Moss authored
* set CookieSpecs/STANDARD on Apache HttpClient for Snowplow * add a comment in the code pointing to the PR * set cookie spec via RequestConfig instead
-
Diogo Mendes authored
-
Jeff Bruemmer authored
* notes from Luiz * refresh cache * typo
-
Howon Lee authored
* rewrite here... * no bq * thinking * conjunction of alias thing... * make sure we dont wanna see it * nit * multi-arity func * fiddly bit on test * add bit to see order by works right * no default breakout true
-
Anton Kulyk authored
-
Alexander Polyankin authored
-
Alexander Polyankin authored
-
Jeff Bruemmer authored
-
Braden Shepherdson authored
This PR handles the other JSON-encoded MBQL snippets I was able to find. These snippets contain `Table` and `Field` IDs, and so are not portable. These fields are expanded during serialization and the IDs replaced with portable references, then converted back in deserialization. Note that the referenced field must already be loaded before it has a valid ID. `serdes-dependencies` defines this order, therefore each entity depends on those tables and fields referenced in its MBQL fragments. The complete set of fields I found to convert: - `Metric.definition` - `Segment.definition` - `DashboardCard.parameter_mappings` - `Card.parameter_mappings`
-