Skip to content
Snippets Groups Projects
This project is mirrored from https://github.com/metabase/metabase. Pull mirroring updated .
  1. Dec 09, 2022
  2. Dec 08, 2022
    • Cam Saul's avatar
      Add `=?` test expression type (#23982) · 43236c0a
      Cam Saul authored
      
      * ≈ [WIP] [ci skip]
      
      * Minor cleanup [ci skip]
      
      * Code cleanup. [ci skip]
      
      * Kondo in CI should fetch library configs
      
      * Bump Methodical version
      
      * Change name to `=?`
      
      * Add `#exactly` reader tag
      
      * Add `#schema` data reader
      
      * Fix dev deps indentation and add `algo.generic`
      
      * Add `approx=`
      
      * Improved version of `#approx`
      
      * Just check in third party Kondo config for now instead of fighting CI
      
      * Update test/metabase/test_runner/assert_exprs/approximately_equal_test.clj
      
      Co-authored-by: default avatarTim Macdonald <tim@metabase.com>
      
      * Address PR feedback and fix sequence comparison
      
      Co-authored-by: default avatarTim Macdonald <tim@metabase.com>
      Unverified
      43236c0a
  3. Dec 06, 2022
  4. Dec 02, 2022
  5. Nov 29, 2022
    • Cam Saul's avatar
      Athena driver (#26301) · a32bb77b
      Cam Saul authored
      * Include Amazon Athena support for dacort/metabase-athena-driver
      
      * Use Metabase Maven repo to fetch Athena driver
      
      * Copy test extensions from Damon's Athena driver repo
      
      * Some code cleanup.
      
      * Move namespaces => metabase.driver.athena
      
      * Clean up the test extensions namespace
      
      * 42 failures, 16 errors
      
      * Fix regex support; disable a few tests
      
      * Minor tweaks
      
      * Fix data-source-name
      
      * Fix :week and :day-of-week impls (mostly): 2 failures, 3 errors
      
      * Fix OFFSET, :week-of-year; skip test that has TIME column
      
      * Add Athena to CircleCI config. Don't wait for Java 11 tests to finish before driver tests.
      
      * ALL TESTS ARE PASSING! <3
      
      * Copy fixes for https://github.com/dacort/metabase-athena-driver/issues/115;
      
       add test
      
      * We don't need to prep source files or fetch dependencies before running backend tests.
      
      * Fix Eastwood error.
      
      * Tweak CircleCI config.
      
      * Fix TIMESTAMP WITH TIME ZONE
      
      * Include the Athena/Redshift repos in the build-drivers deps.edn
      
      * Build and release scripts need to have the :mvn/repos as well
      
      * Revert change that enabled test for Presto
      
      * Un-enable failing test for Snowflake as well.
      
      * moves all is clauses into the test
      
      - previously only the first few tests were being run
      
      * Sort ns in `metabase.driver.athena-test`
      
      * Prevent athena's log4j2.properties file from becoming log config
      
      athena includes log4j2.properties top-level with the properties:
      
      ```
      status = debug
      rootLogger.level=debug
      ```
      
      And this kills our beautifully crafted logging. Set
      "log4j2.configurationFile" in bootstrap to our own log4j2.xml.
      
      Log4j2 looks in a few places for its logging config, the first of which
      is the properties file. So when the jar is loaded, log4j2 considers this
      a logging config change and we lose our logging.
      
      Co-authored-by: default avatarDamon P. Cortesi <d.lifehacker@gmail.com>
      Co-authored-by: default avatarBryan Maass <bryan.maass@gmail.com>
      Co-authored-by: default avatardan sutton <dan@dpsutton.com>
      Unverified
      a32bb77b
    • Ngoc Khuat's avatar
  6. Nov 16, 2022
    • dpsutton's avatar
      Initial support for datetimeDiff (#25722) · 254e73f8
      dpsutton authored
      
      * Initial support for pg for `date-diff`
      
      * Make the useful-dates closer to one day
      
      * Add simple FE stuff
      
      * shorter test bodies
      
      * Ensure we can use datediff functions in arithmetic expressions
      
      * Correctly disable datediff for redshift
      
      * simplify var names
      
      * Support week
      
      * cleanup test
      
      * :datediff -> :datetimediff
      
      * ngoc's suggestions
      
      * Better acceptance test for datetimediff
      
      * sort ns
      
      * embrace the different cases for results
      
      * bigquery day month year
      
      * Reverse args
      
      * Update test
      
      * Centralize tests
      
      * Change postgres day, month, year behaviour
      
      * Refactor keep identity
      
      * Tidy tests
      
      * Tidy
      
      * Fix bigquery week
      
      * Add week tests
      
      * Fix bigquery week
      
      * Change mysql day, month, year behaviour
      
      * Add test for hour, minute, second
      
      * Fix postgres hour minute second
      
      * Formatting
      
      * Fix bigquery hour, minute, second
      
      * Formatting
      
      * Fix postgres timestamptz
      
      * WIP
      
      * Allow literals in datediff clauses
      
      * Uncomment tests
      
      * Fix bigquery when reporting timezone is not UTC
      
      * Linting
      
      * Moving away from dataset based tests
      
      * Add timezone tests for week and tidy
      
      * Remove unused import
      
      * Consolidate tests
      
      * Remove with-time-column dataset
      
      * Remove more-useful-dates defdataset
      
      * Remove redshift driver WIP
      
      * Typo
      
      * Move DatetimeLiteral clause into DateTimeExpressionArg
      
      * Try changing test order
      
      * Remove mt/with-report-timezone-id nil
      
      * Add year report timezone tests
      
      * Rename
      
      * Rename
      
      * Remove unused tables from useful-dates
      
      * Remove useful-dates
      
      * Update helper-text-strings
      
      * Tidy
      
      * Swap order of mt/with-report-timezone-id
      
      * Change with-report-timezone-id; notify databases after running test
      
      * Use temp setting for report-timezone instead
      
      * Update helper-test-strings
      
      * Handle literals in `datetimediff-base-base`
      
      * Update src/metabase/driver/postgres.clj
      
      Co-authored-by: default avatarmetamben <103100869+metamben@users.noreply.github.com>
      
      * Simplify postgres second
      
      * Tidy: prefer hx arithmetic functions
      
      * Drop coercion for string timestamp args for now
      
      * Extract helper
      
      * Revert "Change with-report-timezone-id; notify databases after running test"
      
      This reverts commit 7abb543bed13d9d13295d0e00ad1293a40e40d53.
      
      * Use ->timestamptz
      
      * Use hx/->timestamp
      
      * Rename dataset
      
      * Undo changes to datetime-arithmetics? as these will not match by default
      
      * Use proper format for offset datetime literals
      
      * Shorten datetime literals in tests
      
      * Removing notify-all-databases-updated from report-timezone setting
      
      * Fix datetime-arithmetics test
      
      * Revert "Fix datetime-arithmetics test"
      
      This reverts commit 9141582508170bdbe99c4aa74bc4444e41475be6.
      
      * Revert "Undo changes to datetime-arithmetics? as these will not match by default"
      
      This reverts commit 9cb05f5475b24a78cb89601c918f3440606cc525.
      
      * Add UTC timezone to tests
      
      * Coerce strings to datetimes for ISO formats
      
      * Revert "Removing notify-all-databases-updated from report-timezone setting"
      
      This reverts commit 37356435a2e3981deca9ae76118d857babb2aada.
      
      * Add comment to postgres driver implementation
      
      * Formatting
      
      * Disable datediff from redshift for the moment (for ever?)
      
      * Override redshift driver/database-supports?
      
      * Fix comments mixed up by refactoring
      
      * Fix comments mixed up by refactoring 2
      
      * Tidy comment
      
      * Rename datetimediff to datetime-diff/datetimeDiff
      
      * date-add -> datetime-add
      
      * Linting
      
      * add datetime-subtract to `datetime-arithmetics?`
      
      rework tests a bit as well.
      
      * Update docstring
      
      * Use ->temporal-type and trunc to handle report-timezone for bigquery
      
      * Log errors caught during sync steps (#26306)
      
      * log errors caught during sync steps
      
      * remove accidental extra parens
      
      * Errors combining datetime interval addition with datetime functions (#26279)
      
      * Add failing tests
      
      * Fix failing tests
      
      * Update shared/src/metabase/mbql/util.cljc
      
      Co-authored-by: default avatarNgoc Khuat <qn.khuat@gmail.com>
      
      * datetime-add and datetime subtract should annotate type by col type
      
      * Fix infer-expression-type for datetime-add/subtract with second, minute, hour
      
      * Undo last commit; they actually always return :type/DateTime
      
      * Fix test based on last commit
      
      * Undo unrelated refactor
      
      * Only test drivers that support expressions
      
      * Only test drivers that support expressions, again
      
      * Update tests from legacy mbql
      
      * Change infered-col-type to be a function again, not macro
      
      * Fix test
      
      Co-authored-by: default avatarNgoc Khuat <qn.khuat@gmail.com>
      
      * whitespace
      
      * Remove comments
      
      * Add explanation for datetime_diff
      
      * Refactor: replace cast and add `mt/with-driver :bigquery-cloud-sdk` where report-timezone is relevant
      
      * Add failing tests
      
      * Fix failing tests
      
      * Technically it should be bigquery-type
      
      * Update modules/drivers/bigquery-cloud-sdk/src/metabase/driver/bigquery_cloud_sdk.clj
      
      Co-authored-by: default avatarCam Saul <1455846+camsaul@users.noreply.github.com>
      
      * Move documentation to metabase.mbql.schema
      
      * Update DatetimeDiffUnits
      
      Co-authored-by: default avatarCam Saul <1455846+camsaul@users.noreply.github.com>
      
      * Add error type and optimize case expressions to driver implementations
      
      * refactor for brevity
      
      * Fix error with postgres
      
      * Fix error with postgres
      
      * Handle string literal parsing in wrap-value-literals
      
      * Remove ->timestamptz
      
      * Add comments + TODOs showing arithmetic expressions should return numeric values
      
      * Add explanation of arithmetic expression as docstring
      
      * Add test for normalize-mbql-clause-tokens
      
      * Remove unused import
      
      * Remove unused form
      
      * Switch tests to use attempted-murders dataset
      
      * Undo optimization that broke tests
      
      * Validate non-temporal types for bigquery
      
      * Add error handling for incorrect types
      
      * Fix mysql type checking
      
      * Fix mysql type checking
      
      * invalid-parameter -> invalid-query
      
      * Use date-trunc and extract
      
      * Remove unit error handling
      
      * DatetimeLiteral -> DateOrDatetimeLiteral
      
      * Remove unused binding
      
      * Fix mysql type checking
      
      Co-authored-by: default avatarCallum Herries <hi@callumherries.com>
      Co-authored-by: default avatarCal Herries <39073188+calherries@users.noreply.github.com>
      Co-authored-by: default avatarmetamben <103100869+metamben@users.noreply.github.com>
      Co-authored-by: default avatarNoah Moss <32746338+noahmoss@users.noreply.github.com>
      Co-authored-by: default avatarNgoc Khuat <qn.khuat@gmail.com>
      Co-authored-by: default avatarCam Saul <1455846+camsaul@users.noreply.github.com>
      Unverified
      254e73f8
  7. Nov 09, 2022
  8. Nov 08, 2022
  9. Nov 07, 2022
    • dpsutton's avatar
      Bump woodstox : CVE-2022-40151 (#26269) · 84df58aa
      dpsutton authored
      First seen in trivy report:
      https://github.com/metabase/metabase/pull/26161/checks?check_run_id=9326286850
      
      CVE:
      https://avd.aquasec.com/nvd/cve-2022-40151
      
      xstream: Xstream to serialise XML data was vulnerable to Denial of
      Service attacks High
      Package: com.fasterxml.woodstox:woodstox-core
      Installed Version: 6.2.6
      Vulnerability CVE-2022-40151
      Severity: HIGH
      Fixed Version: 5.4.0, 6.4.0
      
      Bumping deps and comparing `clj -X:deps tree` shows the change only adds
      the new dep top level and no new deps are brought in by the change.
      
      ```
      ❯ diff --unified deps deps-updated
      --- deps	2022-11-07 08:43:21.000000000 -0600
      +++ deps-updated	2022-11-07 08:49:56.000000000 -0600
      @@ -9,6 +9,8 @@
         X org.slf4j/slf4j-api 1.7.25 :use-top
         X org.apache.logging.log4j/log4j-api 2.18.0 :use-top
         X org.apache.logging.log4j/log4j-core 2.18.0 :use-top
      +com.fasterxml.woodstox/woodstox-core 6.4.0
      +  . org.codehaus.woodstox/stax2-api 4.2.1
       joda-time/joda-time 2.10.13
       commons-codec/commons-codec 1.15
       weavejester/dependency 0.2.1
      @@ -285,8 +287,7 @@
         . org.apache.santuario/xmlsec 2.3.0
           X org.slf4j/slf4j-api 1.7.32 :use-top
           X commons-codec/commons-codec 1.15 :use-top
      -    . com.fasterxml.woodstox/woodstox-core 6.2.6
      -      . org.codehaus.woodstox/stax2-api 4.2.1
      +    X com.fasterxml.woodstox/woodstox-core 6.2.6 :use-top
           . jakarta.xml.bind/jakarta.xml.bind-api 2.3.3
             . jakarta.activation/jakarta.activation-api 1.2.2
         . org.opensaml/opensaml-xmlsec-api 3.4.6
      ```
      Unverified
      84df58aa
  10. Oct 20, 2022
    • Cam Saul's avatar
      Initialize Metabase with config from a file (#25986) · 5625fcb1
      Cam Saul authored
      
      * Initialize Metabase from a file
      
      * Code cleanup
      
      * Support optional template tags; more dox
      
      * Finish documentation for ns str
      
      * Force the first User to be an admin
      
      * Tests for DB creation
      
      * Add test to ensure that you can set User password with an {{env ...}} template
      
      * Remove data app permission code (#26016)
      
      * Revert "Use "apps" collection namespace for app collections (#25963)"
      
      This reverts commit a1894202.
      
      * Revert "Implement EE/Pro app permissions (#25764)"
      
      This reverts commit cf9b1bf5.
      
      * Revert "Implement global app permissions for the "All Users" group (#25679)"
      
      This reverts commit 63e950f2.
      
      * Revert "Require data permissions for executing actions (#25784)"
      
      This reverts commit 732bf6b1.
      
      * Revert "Prevent modifying app permissions via the collection endpoint (#25684)"
      
      This reverts commit 5d3f8279.
      
      * Revert "Implement DB specific execution permissions (#25629)"
      
      This reverts commit 141a13ce.
      
      * Revert "Introduce /execution/ permission (#25529)"
      
      This reverts commit 57cbb921.
      
      * Keep the app permission revision table
      
      * [Apps] Remove emitters (#25999)
      
      * [Apps] Remove emitters
      
      Emitters were created when we thought that we would be adding actions to
      multiple places in metabase. Instead we've coalesced to actions being a
      core feature of data-apps and are executed from within them. Attaching
      actions to models, as a form of re-use rather than a form of execution,
      means that actions can still be executed directly from a model, if we
      want to allow that in the future.
      
      * Fix lint errors
      
      * Move perform-action! tests out of the api.
      
      Since we want to keep the implicit bulk functionality but remove those
      endpoints move the tests to the actions namespace.
      
      * Fix linter
      
      * Change driver test to use function instead of endpoint
      
      Co-authored-by: default avatarmetamben <103100869+metamben@users.noreply.github.com>
      Co-authored-by: default avatarCase Nelson <case@metabase.com>
      Unverified
      5625fcb1
  11. Sep 26, 2022
  12. Sep 22, 2022
  13. Sep 19, 2022
    • dpsutton's avatar
      initial prometheus sketch (#25149) · 00530c98
      dpsutton authored
      * initial prometheus sketch
      
      need env variable to start: MB_PROMETHEUS_SERVER_PORT=9191
      
      sample of output:
      
      ```
      '# HELP jvm_threads_current Current thread count of a JVM
      '# TYPE jvm_threads_current gauge
      jvm_threads_current 81.0
      '# HELP jvm_threads_daemon Daemon thread count of a JVM
      '# TYPE jvm_threads_daemon gauge
      jvm_threads_daemon 36.0
      '# HELP jvm_threads_peak Peak thread count of a JVM
      '# TYPE jvm_threads_peak gauge
      jvm_threads_peak 81.0
      '# HELP jvm_threads_started_total Started thread count of a JVM
      '# TYPE jvm_threads_started_total counter
      jvm_threads_started_total 104.0
      '# HELP jvm_threads_deadlocked Cycles of JVM-threads that are in deadlock waiting to acquire object monitors or ownable synchronizers
      '# TYPE jvm_threads_deadlocked gauge
      jvm_threads_deadlocked 0.0
      ```
      
      request:
      
      ```
      ❯ http localhost:9191/metrics
      HTTP/1.1 200 OK
      Content-Length: 7329
      Content-Type: text/plain; version=0.0.4; charset=utf-8
      Date: Wed, 31 Aug 2022 16:13:38 GMT
      Server: Jetty(9.4.48.v20220622)
      
      '# HELP jvm_gc_collection_seconds Time spent in a given JVM garbage collector in seconds.
      '# TYPE jvm_gc_collection_seconds summary
      jvm_gc_collection_seconds_count{gc="G1 Young Generation",} 41.0
      jvm_gc_collection_seconds_sum{gc="G1 Young Generation",} 0.586
      jvm_gc_collection_seconds_count{gc="G1 Old Generation",} 0.0
      jvm_gc_collection_seconds_sum{gc="G1 Old Generation",} 0.0
      '# HELP jvm_threads_current Current thread count of a JVM
      '# TYPE jvm_threads_current gauge
      ```
      
      * Log on unparseable prometheus port
      
      * Clean up prometheus and save test
      
      * typehint
      
      * Jetty collector
      
      * Reset system to nil when shutting down
      
      * c3p0 stats
      
      * Clean up, document, and add tests
      
      * Error message for failure to bind to port
      
      Starting up with prometheus port set to the main webserver port to get
      the error:
      
      ```
      MB_JETTY_PORT=3006 MB_DB_CONNECTION_URI="postgres://..." \
      MB_PROMETHEUS_SERVER_PORT=3006 java -jar locally-built.jar
      ```
      yields the following error:
      
      ```shell
      2022-09-09 10:08:52,000 INFO metabase.core :: Setting up prometheus metrics
      2022-09-09 10:08:52,002 INFO metabase.prometheus :: Starting prometheus metrics collector
      2022-09-09 10:08:52,016 INFO metabase.prometheus :: Starting prometheus metrics web-server on port 3,006
      2022-09-09 10:08:52,036 ERROR metabase.core :: Metabase Initialization FAILED
      clojure.lang.ExceptionInfo: Failed to initialized Prometheus on port 3,006 {:port 3006}
      [stacktrace ...]
      Caused by: java.io.IOException: Failed to bind to 0.0.0.0/0.0.0.0:3006
      [stacktrace ...]
      Caused by: java.net.BindException: Address already in use
      ```
      
      * Test for error message
      
      * Move prometheus to analytics folder
      
      * Str port so log does not add commas to number
      
      eg:
      > Starting prometheus metrics web-server on port 9,191
      
      * make some test functions private
      
      * docstring on defsetting
      
      * align lets
      
      * ns docstring changes
      
      include the env variable for ease of understanding
      
      * sort ns
      
      * remove some reflection warnings
      
      * reorder FE lines due to some new linter
      
      * Cleanup: i18n descriptions, typo, List/of
      
      * bit more concise c3p0 collection
      
      * no longer need the helper `->array`
      
      * clean up doseq for c3p0 measurements
      Unverified
      00530c98
  14. Sep 13, 2022
  15. Sep 07, 2022
  16. Aug 17, 2022
  17. Aug 16, 2022
  18. Aug 12, 2022
    • Cam Saul's avatar
      Enable Kondo for tests (part 1) (#24736) · bc4acbd2
      Cam Saul authored
      * Fix some small things
      
      * Add Kondo to deps.edn to be able to debug custom hooks from REPL
      
      * Fix macroexpansion hook for with-temp* without values
      
      * Test config (WIP)
      
      * More misc fixes
      
      * Disable :inline-def for tests
      
      * More misc fixes
      
      * Fix $ids and mbql-query kondo hooks.
      
      * Fix with-temporary-setting-values with namespaced symbols
      
      * More misc fixes
      
      * Fix the rest of the easy ones
      
      * Fix hook for mt/dataset
      
      * Horrible hack to work around https://github.com/clj-kondo/clj-kondo/issues/1773 . Custom linter for mbql-query macro
      
      * Fix places calling mbql-query with a keyword table name
      
      * Fix the last few errors in test/
      
      * Fix errors in enterprise/test and shared/test
      
      * Fix driver test errors
      
      * Enable linters on CI
      
      * Enable unresolved-namespace linter for tests
      
      * Appease the namespace linter again
      
      * Test fixes
      Unverified
      bc4acbd2
  19. Aug 10, 2022
  20. Aug 05, 2022
    • Reza Lotun's avatar
      Bump postgres to 42.3.6 (#24644) · b5a794e1
      Reza Lotun authored
      Unverified
      b5a794e1
    • Reza Lotun's avatar
      Bump Mariadb driver to 2.7.6 (#24643) · 1a3563fe
      Reza Lotun authored
      Unverified
      1a3563fe
    • dpsutton's avatar
      Fix in-memory logger (#24616) · df165ba1
      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 :coffee: v17.30 on :cloud:  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
      Unverified
      df165ba1
  21. Aug 03, 2022
    • dpsutton's avatar
      Remove deprecated friend library (#24543) · 4069ac4f
      dpsutton authored
      * Remove deprecated friend library
      
      - friend has two functions we used: bcrypt and bcrypt-verify. Easy to
      lift them into our own namespace with attribution
      - uses simple interop on org.mindrot.jbcrypt.BCrypt to achieve these
      - also brings in other stuff we don't need
      
      ```
      com.cemerick/friend 0.2.3
        X org.mindrot/jbcrypt 0.3m :use-top <- all we care about
        X org.clojure/core.cache 0.6.3 :superseded
          X org.clojure/data.priority-map 0.0.2 :parent-omitted
        . org.openid4java/openid4java-nodeps 0.9.6
          X commons-logging/commons-logging 1.1.1 :older-version
          . net.jcip/jcip-annotations 1.0
        . com.google.inject/guice 2.0
          . aopalliance/aopalliance 1.0
      ```
      
      And we already declare a dependency on 0.4 of this lib
      
      ```
      org.mindrot/jbcrypt 0.4
      ```
      
      This means we can remove openid4, google.inject/guice, aopalliance, etc
      and just keep using the same `BCrypt` java class we have been using this
      whole time. Behavior and classfiles are identical. So very low risk
      
      Want to call out a use of
      
      ```clojure
          (when-not api/*is-superuser?*
            (api/checkp (u.password/bcrypt-verify (str (:password_salt user) old_password) (:password user))
              "old_password"
              (tru "Invalid password")))
      ```
      
      This has the same signature of an existing function in `u.password/verify-password`:
      
      ```clojure
      (defn verify-password
        "Verify if a given unhashed password + salt matches the supplied hashed-password. Returns `true` if matched, `false`
        otherwise."
        ^Boolean [password salt hashed-password]
        ;; we wrap the friend/bcrypt-verify with this function specifically to avoid unintended exceptions getting out
        (boolean (u/ignore-exceptions
                   (bcrypt-verify (str salt password) hashed-password))))
      ```
      
      I did not replace it in this PR so that the diff is essentially
      `creds/<fn>` -> `u.password/<fn>` and very easy to structually see what
      is going on.
      
      But totally makes sense to clean up the usages of these in another pass
      
      * sort ns
      
      * simple tests
      Unverified
      4069ac4f
  22. Aug 01, 2022
  23. Jul 28, 2022
    • dpsutton's avatar
      Multi release jar again (#24366) · 1f4bf25f
      dpsutton authored
      * Try multi-release true again in our manifest
      
      Problem statement:
      Luiz packs our partner jars (exasol, starburst, etc.) into our jar so
      they can be "first class" and in cloud. But with the 44 cycle we've run
      into some issues:
      
      ```shell
      /tmp/j via :coffee: v17.30 on :cloud:  metabase-query
      ❯ jar uf 0.44.0-RC1.jar modules/*.jar
      
      ❯ java --version
      openjdk 11.0.14.1 2022-02-08
      OpenJDK Runtime Environment Temurin-11.0.14.1+1 (build 11.0.14.1+1)
      OpenJDK 64-Bit Server VM Temurin-11.0.14.1+1 (build 11.0.14.1+1, mixed mode)
      
      /tmp/j via :coffee: v11.0.14.1 on :cloud:  metabase-query
      ❯ jar uf 0.44.0-RC1.jar modules/*.jar
      java.lang.module.InvalidModuleDescriptorException: Unsupported major.minor version 61.0
      	at java.base/jdk.internal.module.ModuleInfo.invalidModuleDescriptor(ModuleInfo.java:1091)
      	at java.base/jdk.internal.module.ModuleInfo.doRead(ModuleInfo.java:195)
      	at java.base/jdk.internal.module.ModuleInfo.read(ModuleInfo.java:147)
      	at java.base/java.lang.module.ModuleDescriptor.read(ModuleDescriptor.java:2553)
      	at jdk.jartool/sun.tools.jar.Main.addExtendedModuleAttributes(Main.java:2083)
      	at jdk.jartool/sun.tools.jar.Main.update(Main.java:1017)
      	at jdk.jartool/sun.tools.jar.Main.run(Main.java:366)
      	at jdk.jartool/sun.tools.jar.Main.main(Main.java:1680)
      
      ```
      
      Diogo tracked this down with some great sleuthing to an upgrade in our
      graal/js engine from “22.0.0.2” -> “22.1.0". This brought along the
      transitive truffle jar (which is the actual engine powering the js
      engine). The 22.0.0.2 was technically a multi-release jar but it only
      included java 11 sources. The 22.1.0 added java 17 sources in addition
      to the java 11.
      
      And this proves fatal to using the `jar` command. When `"Multi-Release"`
      is set to true, it knows to only look at versions it will need. Lacking
      this, it looks at all of the classes and the class version for 17 is
      61.0 is higher than it knows how to understand and it breaks.
      
      Obvious Solution:
      Set Multi-Release to true. We have done this in the past. On startup we
      have a message logged:
      
      > WARNING: sun.reflect.Reflection.getCallerClass is not supported. This
      > will impact performance.
      
      And setting multi-release can remove this. But when we did that we ended
      up with:
      - https://github.com/metabase/metabase/issues/16380
      - https://github.com/metabase/metabase/pull/17027
      
      That issue describes slowdowns of queries on the order of 0.6 seconds ->
      1.3 seconds. Almost doubling. People reported dashboards timing
      out. Jeff tracked this down to
      
      > Profiling revealed that the calls to Log4jLoggerFactory.getLogger
      > became much slower between the two versions. See attached screenshots.
      
      And this is a pernicious problem that we cannot easily test for.
      
      Lets try again:
      I've set multi-release to true and built a jar with `bin/build`. I
      immediately ran into problems:
      
      ```shell
      ❯ MB_DB_CONNECTION_URI="postgres://user:pass@localhost:5432/compare
      
      " MB_JETTY_PORT=3007 java "$(socket-repl 6007)" -jar multi-release-local.jar
      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
      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
      Exception in thread "main" java.lang.Error: Circular loading of installed providers detected
      	at java.base/java.nio.file.spi.FileSystemProvider.installedProviders(FileSystemProvider.java:198)
      	at java.base/java.nio.file.Path.of(Path.java:206)
      	at java.base/java.nio.file.Paths.get(Paths.java:98)
      	at org.apache.logging.log4j.core.util.Source.toFile(Source.java:55)
      	at org.apache.logging.log4j.core.util.Source.<init>(Source.java:142)
      	at
              org.apache.logging.log4j.core.config.ConfigurationSource.<init>(ConfigurationSource.java:139)
      ```
      
      So hazarded a guess that a bump in the log4j would solve this. And it
      does solve it.
      
      Then profiling some queries against bigquery (just viewing the table) in
      the RC2 and the locally built version with the multi-release:
      
      ```shell
      -- multi-release
      2022-07-27 12:28:00,659 DEBUG middleware.log :: POST /api/dataset 202 [ASYNC: completed] 1.1 s
      2022-07-27 12:28:02,609 DEBUG middleware.log :: POST /api/dataset 202 [ASYNC: completed] 897.9 ms
      2022-07-27 12:28:03,950 DEBUG middleware.log :: POST /api/dataset 202 [ASYNC: completed] 778.1 ms
      
      -- RC non-multi-release
      2022-07-27 12:28:57,633 DEBUG middleware.log :: POST /api/dataset 202 [ASYNC: completed] 1.0 s
      2022-07-27 12:28:59,343 DEBUG middleware.log :: POST /api/dataset 202 [ASYNC: completed] 912.9 ms
      2022-07-27 12:29:02,328 DEBUG middleware.log :: POST /api/dataset 202 [ASYNC: completed] 808.6 ms
      ```
      So times seem very similar.
      
      ============
      
      Proper benching:
      
      using criterium
      
      ```shell
      MB_JETTY_PORT=3008 java "$(socket-repl 6008)" -cp "/Users/dan/.m2/repository/criterium/criterium/0.4.6/criterium-0.4.6.jar":0.39.2.jar metabase.core
      ```
      
      `(bench (log/warn "benching"))`
      
      Summary:
      39.2:          21.109470 µs
      RC2:           4.975204 µs
      multi-release: 7.673965 µs
      
      These flood the consoles with logs
      
      ```
      Older release: 39.2
      
      user=> (bench (log/warn "benching"))
      Evaluation count : 2886240 in 60 samples of 48104 calls.
                   Execution time mean : 21.109470 µs
          Execution time std-deviation : 567.271917 ns
         Execution time lower quantile : 20.171870 µs ( 2.5%)
         Execution time upper quantile : 22.429557 µs (97.5%)
                         Overhead used : 6.835913 ns
      
      Found 5 outliers in 60 samples (8.3333 %)
      	low-severe	 4 (6.6667 %)
      	low-mild	 1 (1.6667 %)
       Variance from outliers : 14.1886 % Variance is moderately inflated by outliers
      
      =============================================
      
      RC2:
      
      user=> (bench (log/warn "benching"))Evaluation count : 12396420 in 60 samples of 206607 calls.
                   Execution time mean : 4.975204 µs
          Execution time std-deviation : 521.769687 ns
         Execution time lower quantile : 4.711607 µs ( 2.5%)
         Execution time upper quantile : 6.404317 µs (97.5%)
                         Overhead used : 6.837290 ns
      
      Found 5 outliers in 60 samples (8.3333 %)
      	low-severe	 2 (3.3333 %)
      	low-mild	 3 (5.0000 %)
       Variance from outliers : 72.0600 % Variance is severely inflated by outliers
      
      =============================================
      
      Proposed Multi-Release
      
      user=> (bench (log/warn "benching"))
      Evaluation count : 7551000 in 60 samples of 125850 calls.
                   Execution time mean : 7.673965 µs
          Execution time std-deviation : 201.155749 ns
         Execution time lower quantile : 7.414837 µs ( 2.5%)
         Execution time upper quantile : 8.138010 µs (97.5%)
                         Overhead used : 6.843981 ns
      
      Found 1 outliers in 60 samples (1.6667 %)
      	low-severe	 1 (1.6667 %)
       Variance from outliers : 14.1472 % Variance is moderately inflated by outliers
      
      ```
      
      `(bench (log/info "benching info"))`
      
      This does not hit a console so is a no-op.
      
      Summary:
      39.2:          11.534614 µs
      RC2:           98.408357 ns
      multi-release: 2.236756 µs
      
      ```
      =============================================
      39.2:
      
      user=> (bench (log/info "benching info"))
      Evaluation count : 5223480 in 60 samples of 87058 calls.
                   Execution time mean : 11.534614 µs
          Execution time std-deviation : 57.756163 ns
         Execution time lower quantile : 11.461502 µs ( 2.5%)
         Execution time upper quantile : 11.657644 µs (97.5%)
                         Overhead used : 6.835913 ns
      
      Found 3 outliers in 60 samples (5.0000 %)
      	low-severe	 2 (3.3333 %)
      	low-mild	 1 (1.6667 %)
       Variance from outliers : 1.6389 % Variance is slightly inflated by outliers
      
      =============================================
      
      RC2:
      
      user=> (bench (log/info "benching info"))Evaluation count : 574427220 in 60 samples of 9573787 calls.
                   Execution time mean : 98.408357 ns
          pExecution time std-deviation : 1.792214 ns
         Execution time lower quantile : 96.891477 ns ( 2.5%)
         Execution time upper quantile : 103.394664 ns (97.5%)
                         Overhead used : 6.837290 ns
      
      Found 8 outliers in 60 samples (13.3333 %)
      	low-severe	 3 (5.0000 %)
      	low-mild	 5 (8.3333 %)
       Variance from outliers : 7.7881 % Variance is slightly inflated by outliers
      
      =============================================
      
      Multi-release:
      
      user=> (bench (log/info "benching info"))Evaluation count : 26477700 in 60 samples of 441295 calls.
                   Execution time mean : 2.236756 µs
          Execution time std-deviation : 15.412356 ns
         Execution time lower quantile : 2.212301 µs ( 2.5%)
         Execution time upper quantile : 2.275434 µs (97.5%)
                         Overhead used : 6.843981 ns
      
      Found 3 outliers in 60 samples (5.0000 %)
      	low-severe	 3 (5.0000 %)
       Variance from outliers : 1.6389 % Variance is slightly inflated by outliers
       ```
      
      * bump graal/js
      
      * Custom MB log factory (#24369)
      
      * Custom MB log factory
      
      * Write stupid code to appease stupid Eastwood
      
      * `ns-name` already calls `the-ns` on its argument.
      
      * More code cleanup
      
      * Improved code
      
      * Remove NOCOMMIT
      
      * empty commit to trigger CI
      
      Co-authored-by: default avatarCam Saul <1455846+camsaul@users.noreply.github.com>
      Unverified
      1f4bf25f
  24. Jul 27, 2022
    • dpsutton's avatar
      Downgrade graal to prevent multi-release issues (#24357) · b40e90d7
      dpsutton authored
      The jar worked fine except when trying to add partner jars (exasol,
      starburst, etc)
      
      ```shell
      ❯ java --version
      openjdk 11.0.14.1 2022-02-08
      OpenJDK Runtime Environment Temurin-11.0.14.1+1 (build 11.0.14.1+1)
      OpenJDK 64-Bit Server VM Temurin-11.0.14.1+1 (build 11.0.14.1+1, mixed mode)
      
      /tmp/j via :coffee: v11.0.14.1 on :cloud:  metabase-query
      ❯ jar uf 0.44.0-RC1.jar modules/*.jar
      java.lang.module.InvalidModuleDescriptorException: Unsupported major.minor version 61.0
      	at java.base/jdk.internal.module.ModuleInfo.invalidModuleDescriptor(ModuleInfo.java:1091)
      	at java.base/jdk.internal.module.ModuleInfo.doRead(ModuleInfo.java:195)
      	at java.base/jdk.internal.module.ModuleInfo.read(ModuleInfo.java:147)
      	at java.base/java.lang.module.ModuleDescriptor.read(ModuleDescriptor.java:2553)
      	at jdk.jartool/sun.tools.jar.Main.addExtendedModuleAttributes(Main.java:2083)
      	at jdk.jartool/sun.tools.jar.Main.update(Main.java:1017)
      	at jdk.jartool/sun.tools.jar.Main.run(Main.java:366)
      	at jdk.jartool/sun.tools.jar.Main.main(Main.java:1680)
      ```
      
      The 22.1.0 graal/js requires a similarly versioned graal/truffle which
      is multi-release but includes on versions/11 class files. The upgraded
      one includes versions/17 and since our uberjar is not multi-release,
      when running it on java 11 it rejects handling class version 61.0 (java
      17) files. If the uberjar were multi-release it would know to select the
      versions it wanted.
      Unverified
      b40e90d7
  25. Jul 25, 2022
  26. Jul 15, 2022
  27. Jul 06, 2022
  28. Jun 29, 2022
  29. Jun 16, 2022
  30. Jun 08, 2022
  31. Jun 01, 2022
  32. May 17, 2022
    • Bryan Maass's avatar
      bumps outdated deps versions to be current + drop support for java 8 (#22567) · c1b73ed6
      Bryan Maass authored
      * bumps outdated deps versions to be current
      
      * un-upgrade h2 and jetty
      
      * un-upgrade joda-time and kixi/stats
      
      * drop Java 8 support in circle CI config
      
      - things that used to rely on be-tests-java-8-ee now rely on be-tests-java-11-ee
      
      * remove java 8 from github health check matrix
      
      * revert toucan to 1.17.0
      
      * revert mariadb java client to 2.7.5
      
      * Back to 18, and handle new behavior
      
      toucan used to just look in *.models.<model-name> for models and just
      give up apparently. I made a feature that toucan will look in a model
      registry to create models rather than using the convention
      https://github.com/metabase/toucan/commit/762ad69defc1477423fa9423e9320ed318f7cfe7
      
      
      but now we're getting errors in these tests about maps vs models.
      
      ```clojure
      revision_test.clj:154
      Check that revisions+details pulls in user info and adds description
      expected: [#metabase.models.revision.RevisionInstance{:is_reversion false,
                                                            :is_creation false,
                                                            :message nil,
                                                            :user
                                                            {:id 1,
                                                             :common_name "Rasta Toucan",
                                                             :first_name "Rasta",
                                                             :last_name "Toucan"},
                                                            :diff
                                                            {:o1 nil, :o2 {:name "Tips Created by Day", :serialized true}},
                                                            :description nil}]
        actual: (#metabase.models.revision.RevisionInstance{:description nil,
                                                            :is_creation false,
                                                            :is_reversion false,
                                                            :user
                                                            {:id 1,
                                                             :first_name "Rasta",
                                                             :last_name "Toucan",
                                                             :common_name "Rasta Toucan"},
                                                            :message nil,
                                                            :diff
                                                            {:o1 nil,
                                                             :o2
                                                             #metabase.models.revision_test.FakedCardInstance{:name
                                                                                                              "Tips Created by Day",
                                                                                                              :serialized
                                                                                                              true}}})
      ```
      
      The only difference here is `:o2` is a
      `metabase.models.revision_test.FakedCardInstance` but still has the same
      keys, `:name`, and `:serialized`. So all is well, we're just able to
      make the model.
      
      So a few different fixes. Some are use `partial=` which doesn't care
      about record/map distinction. Some are just make the model, and some are
      turning them into maps for revision strings (which more closely mimics
      what the real revision stuff does):
      
      ```clojure
      (defn default-diff-map
        "Default implementation of `diff-map` which simply uses clojures `data/diff` function and sets the keys `:before` and `:after`."
        [_ o1 o2]
        (when o1
          (let [[before after] (data/diff o1 o2)]
            {:before before
             :after  after})))
      
      (defn default-diff-str
        "Default implementation of `diff-str` which simply uses clojures `data/diff` function and passes that on to `diff-string`."
        [entity o1 o2]
        (when-let [[before after] (data/diff o1 o2)]
          (diff-string (:name entity) before after)))
      ```
      
      So all in all this change impacts nothing in the app itself, because
      those models follow convention and are correct in
      `metabase.models.<model-name>` and are thus "modelified":
      
      ```clojure
      revision-test=> (revision/revisions Card 1)
      [#metabase.models.revision.RevisionInstance{:is_creation true,
                                                  :model_id 1,
                                                  :id 1,
                                                  :is_reversion false,
                                                  :user_id 2,
                                                  :timestamp #object[java.time.OffsetDateTime
                                                                     "0x77e037f"
                                                                     "2021-10-28T15:10:19.828539Z"],
                                                  :object #metabase.models.card.CardInstance
                                                  {:description nil,
                                                   :archived false,
                                                   :collection_position nil,
                                                   :table_id 5,
                                                   :database_id 2,
                                                   :enable_embedding false,
                                                   :collection_id nil,
                                                   :query_type :query,
                                                   :name "ECVYUHSWQJYMSOCIFHQC",
                                                   :creator_id 2,
                                                   :made_public_by_id nil,
                                                   :embedding_params nil,
                                                   :cache_ttl 1234,
                                                   :dataset_query {:database 2,
                                                                   :type :query,
                                                                   :query {:source-table 5,
                                                                           :aggregation [[:count]]}},
                                                   :id 1,
                                                   :display :scalar,
                                                   :visualization_settings {:global {:title nil}},
                                                   :dataset false,
                                                   :public_uuid nil},
                                                  :message nil,
                                                  :model "Card"}]
      ```
      
      so the model/no-model is just arbitrary distinction in the test. All of
      them in the actual app are turned into models:
      
      ```clojure
      (defn- do-post-select-for-object
        "Call the appropriate `post-select` methods (including the type functions) on the `:object` this Revision recorded.
        This is important for things like Card revisions, where the `:dataset_query` property needs to be normalized when
        coming out of the DB."
        [{:keys [model], :as revision}]
        ;; in some cases (such as tests) we have 'fake' models that cannot be resolved normally; don't fail entirely in
        ;; those cases
        (let [model (u/ignore-exceptions (db/resolve-model (symbol model)))]
          (cond-> revision
          ;; this line would not find a model previously for FakedCard and
          ;; just return the map. But now the registry in toucan _finds_ the
          ;; model defintion and returns the model'd map
            model (update :object (partial models/do-post-select model)))))
      
      (u/strict-extend (class Revision)
        models/IModel
        (merge models/IModelDefaults
               {:types       (constantly {:object :json})
                :pre-insert  pre-insert
                :pre-update  (fn [& _] (throw (Exception. (tru "You cannot update a Revision!"))))
                :post-select do-post-select-for-object}))
      ```
      
      * try using mssql-jdbc 10.2.1.jre11
      
      - Important that we get off the jre8 version
      
      * various fixes that needn't be reverted
      
      * Revert "various fixes that needn't be reverted"
      
      This reverts commit 2a820db0743d0062eff63366ebe7bc78b852e81f.
      
      * go back to using circle ci's java 11 docker image
      
      * java-16 (?) -> java-17
      
      * Revert "go back to using circle ci's java 11 docker image"
      
      This reverts commit b9b14c535a689f701d7e2541081164288c988c4e.
      
      Co-authored-by: default avatardan sutton <dan@dpsutton.com>
      Unverified
      c1b73ed6
  33. May 12, 2022
Loading