Skip to content
Snippets Groups Projects
  • Bryan Maass's avatar
    c1b73ed6
    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>
    bumps outdated deps versions to be current + drop support for java 8 (#22567)
    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>
Code owners
Assign users and groups as approvers for specific file changes. Learn more.