Skip to content
Snippets Groups Projects
This project is mirrored from https://github.com/metabase/metabase. Pull mirroring updated .
  1. May 24, 2022
    • Case Nelson's avatar
      Put model refers on their own line in cmd.copy (#22913) · 56a8eb0e
      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.
      56a8eb0e
    • Case Nelson's avatar
      Add anchor time to persistence schedule (#22827) · dbf40b72
      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
      dbf40b72
    • Noah Moss's avatar
      7915ba3a
  2. May 23, 2022
    • dpsutton's avatar
      Fix flaky test in randomized schedules (#22865) · 72a3cf66
      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™
      72a3cf66
    • dpsutton's avatar
      Put `default-query-constraints` behind a function for setting (#22792) · b08cbcca
      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.
      b08cbcca
    • dpsutton's avatar
      Handle `nil` cache during initial cache population (#22779) · 6ca66e6e
      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)`.
      6ca66e6e
  3. May 19, 2022
    • Case Nelson's avatar
      Change permission on GET /api/persist (#22864) · 2ef1c2ad
      Case Nelson authored
      Since this information is used in the model cache logging screen
      accessible by monitoring users, :monitoring rather than :setting
      permission is the correct one.
      2ef1c2ad
    • Cam Saul's avatar
      `defsetting` setter functions should end in `!` (#22800) · 9dd96b3c
      Cam Saul authored
      * `defsetting` setter functions should end in `!`
      
      * Fix typo
      
      * Update .clj-kondo/hooks/metabase/models/setting.clj
      
      * Fix clj-kondo for Toucan defmodel not emitting a docstring
      
      * Remove `^:private` metadata on a couple of Settings since it makes Eastwood fussy
      9dd96b3c
  4. May 18, 2022
  5. May 17, 2022
    • adam-james's avatar
      Whitelabelling users font family font setting (#22791) · 856c46e2
      adam-james authored
      
      * Add font utilities to derive list of available fonts from font dirs
      
      This PR adds two defsettings to facilitate Enterprise users' ability to set the font for their instance. The
      'available-fonts' can be grabbed by the frontend to populate the dropdown menu. The 'application-font' setting can be
      written by the frontend to save the chosen font.
      
      * Adding some fonts and font selector
      
      * Send  (("dirname", "font name"),) to frontend
      
      Instead of only sending a list of names, send a tuple of `[directory-name, Display Name]`
      
      * Adding half of the fonts
      
      * Font list derived from font directory/font file names
      
      This is an attempt to derive the 'Display Name' of a font by comparing the directory name to the file names. '_' is
      replaced with spaces, but '-' exists in the 'Lato' font directory name, and we may have naming confusion in the
      future. Basically, I want to be liberal with what font directory/file names we can receive.
      
      Not certain this is the right approach yet, but it will work and correctly provides names based on the fonts available.
      
      * Adding remaining fonts
      
      * Using Default Font family in visual components
      
      * Small change to fix some lint warnings
      
      * Removing Noto Sans JP
      
      * Move logging
      
      * Alter hardcoded font path
      
      * Added cypress test
      
      * Change Cypress Test to look for correct text
      
      * Added unit tests for fonts.clj, and simplified creating display name
      
      The display name approach is simple -> underscores become spaces, and names are split on dashes/first word is taken
      from the split.
      
      This might not be robust to future font installations, but will be a good v1
      
      * adding visual test
      
      * fixing cypress test
      
      * Actually add the unit tests
      
      * Fix lint problems
      
      * Add setter to application-font to prevent invalid font setting
      
      * Addressing PR review points
      
      - fixed 'normalize-font-dirname' description
      - 'contains-font-file?' returns true/false
      - 'available-fonts-test' now properly uses 'is'
      
      * Make font-path private
      
      * Lint error.
      
      * Modified fonts to use java.io/resource to hopefully work in a jar
      
      * Simplified Lato folder name
      
      * PR Cleanup
      
      * Address some backend feedback
      
      * Lint error from reflection with .toString on a path
      
      I removed the explicit .toString inside the lamdba functino and str/includes still works.
      
      * no longer need u.files in test ns
      
      Co-authored-by: default avatarNick Fitzpatrick <nick@metabase.com>
      856c46e2
    • Case Nelson's avatar
      Add extra logging for persisted models (#22784) · bf5ca940
      Case Nelson authored
      Add inidividual logs for each model.
      Add final log that includes successes and errors.
      bf5ca940
    • metamben's avatar
      Prevent do-with-timeout from throwing Throwable return values (#22728) · 3521de88
      metamben authored
      Since the documentation of do-with-timeout doesn't mention that return
      values that are instances of Throwable will be thrown instead of being
      returned as other values, it's better not to do this.
      3521de88
    • Bryan Maass's avatar
      Revert backend api actions (#22755) · 3acbe649
      Bryan Maass authored
      * Revert "Revert new precondition"
      
      This reverts commit 1f97a4d6ccfdaa92ff82fd0256f08f3512c3c5e7.
      
      * Revert "plug in and alter tests to use with-actions-test-data"
      
      This reverts commit 2f33cb47b700b1da106adf54d41928c5956978d0.
      
      * Revert "Add Database-local Setting for enabling Actions for a specific Database"
      
      This reverts commit a52f8d0a478a31bb1f1b4a6693abf8f8e658d9cf.
      
      * Revert "Make sure api.actions is loaded"
      
      This reverts commit 7d7b913a7a0d64cbe2b6907f389d994a1130c0f5.
      
      * Revert "Add `experimental-enable-actions` feature flag (#22559)"
      
      This reverts commit 9b313752.
      
      * Revert "SQL JDBC Delete Row Action (#22608)"
      
      This reverts commit d6864f38.
      
      * Remove actions api docs
      
      * Revert "Destructible version of `test-data` dataset for testing writeback actions (#22692)"
      
      This reverts commit 15c57d9a.
      
      * keep around a few misc improvements
      
      * removes precondition
      
      - {:pre [(nil? (namespace symb))]}
      
      * various fixes that needn't be reverted
      3acbe649
    • Howon Lee's avatar
  6. May 16, 2022
    • Case Nelson's avatar
      Persist auto enable (#22756) · c5a31c48
      Case Nelson authored
      
      * Auto enable persistence for models
      
      When persistence is turned on for a db, we want to enable persistence
      caching for all models in the db.
      
      We do this by finding any models without a PersistentInfo at the top of
      the scheduled refresh task and creating one that will get picked up by
      the refresh.
      
      This necessitated introducing another "off" state on PersistedInfo that
      will get set from the front end, manually disabling persistence on a
      model.  This turns PersistedInfo into a marker so that when the refresh
      task runs again, these models will not be turned back on.
      
      The prune job will prune "off" or "deletable" PersistedInfo. Since we don't
      have a second "off-ing" state; the prune job will "drop if exists" the
      cache table each time. This may need to change.
      
      * Cherry-pick persist-refresh changes from persist-refresh-fail-email
      
      * Ready models when enabling persistence on db
      
      * Handle automatic model persistence in Tools table
      
      * Address review: insert-many instead of  doseq insert
      
      Co-authored-by: default avatarAnton Kulyk <kuliks.anton@gmail.com>
      c5a31c48
    • Case Nelson's avatar
      Persist model statement_timeout (#22690) · 9b949bc6
      Case Nelson authored
      * Persist model statement_timeout
      
      In order to safeguard against unknowingly or accidentally large models
      being persisted, we can leverage the postgres `statement_timeout`
      setting.
      
      This way, if a particular statement (likely `create table as`) takes too
      long to run, the database will kill it for us and reclaim resources.
      
      The timeout will be the lower of 10 minutes and an existing
      statement_timeout. The thinking here is that if an admin explicitly
      limitted metabase's statement runtime we shouldn't exceed it.
      
      * Rename query to avoid shadow problems
      9b949bc6
    • Case Nelson's avatar
      Add remarks to persistence ddl statements (#22757) · a662e90e
      Case Nelson authored
      Simply adds `-- Metabase\n` to the front of ddl queries. Eventually this
      should turn into a multimethod like `qp.util/query->remark` but until we
      write another ddl driver this will work.
      a662e90e
    • adam-james's avatar
      Add font utilities to derive list of available fonts from font dirs (#22612) · f106ebfd
      adam-james authored
      
      * Add font utilities to derive list of available fonts from font dirs
      
      This PR adds two defsettings to facilitate Enterprise users' ability to set the font for their instance. The
      'available-fonts' can be grabbed by the frontend to populate the dropdown menu. The 'application-font' setting can be
      written by the frontend to save the chosen font.
      
      * Adding some fonts and font selector
      
      * Send  (("dirname", "font name"),) to frontend
      
      Instead of only sending a list of names, send a tuple of `[directory-name, Display Name]`
      
      * Adding half of the fonts
      
      * Font list derived from font directory/font file names
      
      This is an attempt to derive the 'Display Name' of a font by comparing the directory name to the file names. '_' is
      replaced with spaces, but '-' exists in the 'Lato' font directory name, and we may have naming confusion in the
      future. Basically, I want to be liberal with what font directory/file names we can receive.
      
      Not certain this is the right approach yet, but it will work and correctly provides names based on the fonts available.
      
      * Adding remaining fonts
      
      * Using Default Font family in visual components
      
      * Removing Noto Sans JP
      
      * Small change to fix some lint warnings
      
      * Move logging
      
      * Alter hardcoded font path
      
      * Added cypress test
      
      * Change Cypress Test to look for correct text
      
      * adding visual test
      
      * fixing cypress test
      
      * Added unit tests for fonts.clj, and simplified creating display name
      
      The display name approach is simple -> underscores become spaces, and names are split on dashes/first word is taken
      from the split.
      
      This might not be robust to future font installations, but will be a good v1
      
      * Actually add the unit tests
      
      * Fix lint problems
      
      * Add setter to application-font to prevent invalid font setting
      
      * Addressing PR review points
      
      - fixed 'normalize-font-dirname' description
      - 'contains-font-file?' returns true/false
      - 'available-fonts-test' now properly uses 'is'
      
      * Make font-path private
      
      * Lint error.
      
      * PR Cleanup
      
      Co-authored-by: default avatarNick Fitzpatrick <nick@metabase.com>
      f106ebfd
    • Noah Moss's avatar
  7. May 13, 2022
    • Case Nelson's avatar
      Update persist model api permissions for EE (#22601) · f302d582
      Case Nelson authored
      * Update persist model api permissions for EE
      
      On OSS, only Admins can enable model cache in `Settings/Cache`,
      enable database model cache in `Settings/Databases` and cache individual models.
      
      On EE/Pro, users with `Settings access` application permissions can enable model
      cache in `Settings/Cache` and users with `Manage database` can enable model
      cache for a database.
      
      * Add tests for application permissions
      f302d582
    • metamben's avatar
      Replace "" with nil for Postgres UUIDs (#22667) · 2f208ff1
      metamben authored
      UUIDs are treated as strings and when checking is-empty/not-empty, we compare
      them against the empty string.
      But an empty string is not a valid UUID, so this comparison is useless for
      Postgres where a UUID field cannot have such a value. Assuming that a UUID
      field is only ever compared to the empty string in the context of
      is-empty/non-empty checks, replacing it with nil is OK.
      2f208ff1
    • dpsutton's avatar
      Record startup times (#22707) · ae032bb0
      dpsutton authored
      * Record startup times
      
      updated `dev/start!`. `mb.core/init!` itself sets up the db, loads
      plugins, and sets the init status so those were superfluous.
      
      * clean up dev namespace
      
      * testing doesn't call init so set a value
      ae032bb0
    • Cam Saul's avatar
      SQL JDBC Delete Row Action (#22608) · d6864f38
      Cam Saul authored
      
      * SQL JDBC Delete Action [ci skip]
      
      * Add note about parsing values [ci skip]
      
      * transition to using mbql for actions/row/:action
      
      - TODO: how to setup a qp store?
      - TODO: better way to keywordize filters?
      
      * some cleanup [ci skip]
      
      * better message on delet statement conflicts
      
      - and a test
      
      * mt/with-everything-store -> seed store manually [ci skip]
      
      * fix some test input
      
      * get more tests passing
      
      * add more validation tests
      
      * privatize catch-throw
      
      * plug in and alter tests to use with-actions-test-data
      
      Co-authored-by: default avatarBryan Maass <bryan.maass@gmail.com>
      d6864f38
    • dpsutton's avatar
      Don't reschedule syncs on startup (#22680) · a6c1b33c
      dpsutton authored
      * Don't reschedule syncs on startup
      
      Previously when we started up, for each database we compare its current
      schedule to its schedule in Quartz, and update the triggers if they were
      different. But we handle this change on db inserts and db updates so
      this is completely unnecessary. We did this back when quartz was in
      memory only and it remained when we persisted.
      
      We can remove this complexity. On an app-db with 400 postgres databases,
      this takes 46 seconds in the old world and appears as just another log
      statement now.
      
      Reports from the wild are even more distressing: the quartz tables got
      full of dead tuples causing queries to the triggers to take a long
      time. They also had 500+ databases. We query all triggers and linear
      search for each database and this process exploded up to as much as 8
      hours. Now we are not doing all of this extra work and startup should be
      quick again.
      
      * clj-kondo ignore unused private var
      
      * appease ns linter
      
      * remove clojure.core/binding and unused bindings
      
      * Rename `default-schedules` -> `default-randomized-schedule`
      
      makes it a bit more clear what's going on
      
      * Put randomizer in transducer over reducible query; tests
      
      basically a `run!` over a reducible query that also does a count for
      logging. Tests now check the three options:
      - default schedules, updated
      - non-default schedules, not updated timestamp verified that it is
      identical.
      - default schedules but user controls schedule: not updated, timestamp
      verified that it is identical.
      
      * Prevent randomized schedules from returning a default value
      
      it actually doesn't matter in the application but the tests are
      asserting that the newly randomized schedule is not one of the defaults
      it was changed from and that can fail sometimes. Doesn't matter
      logically but no biggie to simplify the tests.
      
      Otherwise they fail in (1/60)+(1/24) test runs which is actually
      decently frequently with 26 test suites per commit
      a6c1b33c
    • Jeff Bruemmer's avatar
      78981c9c
  8. May 12, 2022
  9. May 11, 2022
    • Case Nelson's avatar
      Make persisted models use "create table as" (#22640) · 9e38acb0
      Case Nelson authored
      * Make persisted models use "create table as"
      
      There are outstanding issues with database type erasure.
      
      If we try to recreate table columns based on the saved
      definition we'll get types such as `type/Structured` that
      could be `json`, `xml`, etc. Perhaps worse, consider 'citext' that we
      see as `type/Text`. If they have queries on the model such as
      `where email = 'JOE@example.com'` that would start to return different
      results if we created the table with `email text` rather than
      `email citext`.
      
      By using `create table ... as select ....`, we avoid this issue as the db types
      selected will match the db types queried.
      
      * Remove unused require
      
      * Missed validatiy check needs a query to create table as
      9e38acb0
    • Case Nelson's avatar
      Double check state in persisted model jobs (#22565) · e78c7c64
      Case Nelson authored
      These loops could be long running, so before removing or adding a cache
      we should double check that the state of the persisted info is still
      valid for the job.
      e78c7c64
    • Case Nelson's avatar
      Only apply persistence substitution when there's no sandboxing (#22501) · faf1b90b
      Case Nelson authored
      * Only apply persistence substitution when there's no sandboxing
      
      * Added test for sandboxed persistence
      
      * Recursively apply persisted-info/native queries
      
      * Move test into permissions test that has ee code
      faf1b90b
    • dpsutton's avatar
      Verify active database for `GET api/health` (#22606) · 0064e01c
      dpsutton authored
      * Verify active database for `GET api/health`
      
      Previously unconditionally responded with 200, status "ok" if we were
      done initializing, even when we had 1500 stale db connections
      somehow (:scream:).
      
      Now attempt to connect to the database:
      
      ```shell
      ❯ http get localhost:3000/api/health -pb
      {
          "status": "ok"
      }
      
      ❯ http get localhost:3000/api/health
      HTTP/1.1 503 Service Unavailable
      ...
      {
          "status": "Error getting db connection"
      }
      ```
      
      and logging to the server
      ```
      2022-05-10 22:05:16,911 WARN server.routes :: Error in api/health database check
      java.lang.ArithmeticException: Divide by zero
      	at clojure.lang.Numbers.divide(Numbers.java:190)
      	at clojure.lang.Numbers.divide(Numbers.java:3911)
      	at metabase.server.routes$fn__122320.invokeStatic(routes.clj:51)
      ```
      
      * Reuse `sql-jdbc.conn/can-connect-with-spec?` for api/health
      0064e01c
  10. May 10, 2022
Loading