Arvados

Updates at a9cc40fb09724af92bff78ef560011b480232b25
Test run: https://ci.curoverse.com/job/developer-run-tests/606/

Ready for review.

For some reason, your structure.sql added "Tablespace" to every comment, what version of Postgres are you using?

(This creates enough noise in the diff I had trouble finding what actually changed)

- -- Name: api_client_authorizations; Type: TABLE; Schema: public; Owner: -
+ -- Name: api_client_authorizations; Type: TABLE; Schema: public; Owner: -; Tablespace: 

Rest look good.

Fixed the issue by upgrading to PostgreSQL 9.6 (I was running both 9.4 & 9.6 after upgrading to stretch, as suggested by Peter)
Rebased - b0d7f37097e8b5c56bfbfa79a134a8074789ebb4
Test run: https://ci.curoverse.com/job/developer-run-tests/610/

• Is one of these redundant? If not, why not?

add_column :collections, :storage_classes_desired, :jsonb, :default => ["default"]
add_column :collections, :storage_classes_confirmed, :jsonb, :default => []

  before_validation :default_storage_classes, on: :create

  def default_storage_classes
    if self.storage_classes_desired.nil? || self.storage_classes_desired.empty?
      self.storage_classes_desired = ["default"]
    end
    self.storage_classes_confirmed ||= []
  end

• I don't think we have used jsonb Array columns before (only Hash columns). Can you investigate which Arvados filters work with jsonb Array columns?

• We should probably validate that the elements of storage_classes_desired and storage_classes_confirmed are only non-empty strings.

Updates at 281fe0c40c16aacd82cd1ca122e38aa3a3854f2e
Test run: https://ci.curoverse.com/job/developer-run-tests/619/

• Removed default value setting on the model code.
• Added validation on list element types (strings), with relevant tests.
• Checked that 'exists' filter work on jsonb array by adding some tests.

Pending: Some workbench tests are failing due to storage_classes_desired being empty, investigating why this happens.

Updates at 05ececf63
Test run: https://ci.curoverse.com/job/developer-run-tests/620/

Added back the default value setting code on Collection's model as some workbench tests were failing. The default values on the schema will be useful for database migrations with already existing collections.

Lucas Di Pentima wrote:

Updates at 05ececf63
Test run: https://ci.curoverse.com/job/developer-run-tests/620/

Added back the default value setting code on Collection's model as some workbench tests were failing. The default values on the schema will be useful for database migrations with already existing collections.

This LGTM