Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Update Flipper #4489

Open
wants to merge 12 commits into
base: develop
Choose a base branch
from
+96 −69
Open

Update Flipper #4489

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
12 commits
Select commit Hold shift + click to select a range
3362789
build: update flipper to v1
StephenHulme Nov 6, 2024
a67bfb5
docs (readme): add feature-flag details
StephenHulme Nov 6, 2024
fd26e4d
docs (readme): update table of contents
StephenHulme Nov 6, 2024
a21afed
docs (readme): update copyright footer
StephenHulme Nov 6, 2024
809fbd9
fix: update db schema for flipper changes
StephenHulme Nov 6, 2024
5d2e8fd
style: lint
StephenHulme Nov 6, 2024
806916c
docs (readme): put each sentence on own line
StephenHulme Nov 11, 2024
bc06e6d
build: pin flipper to version 1.x
StephenHulme Nov 12, 2024
240311f
Merge branch 'develop' into sh51/update-flipper
StephenHulme Nov 12, 2024
9093312
build: hide cucumber publish message
StephenHulme Nov 12, 2024
b0b82ef
fix: attempt to solve pool thread blocking
StephenHulme Nov 12, 2024
7ed4182
fix: use rails boolean converter to handle truthiness
StephenHulme Nov 21, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 3 additions & 3 deletions Gemfile
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -119,9 +119,9 @@ group :default do
gem 'cancancan'

# Feature flags
gem 'flipper', '~> 0.25.0'
gem 'flipper-active_record', '~> 0.25.0'
gem 'flipper-ui', '~> 0.25.0'
gem 'flipper', '~> 1.0'
gem 'flipper-active_record', '~> 1.0'
gem 'flipper-ui', '~> 1.0'
end

group :development do
Expand Down
26 changes: 15 additions & 11 deletions Gemfile.lock
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -210,15 +210,17 @@ GEM
factory_bot (~> 6.4)
railties (>= 5.0.0)
ffi (1.16.3)
flipper (0.25.4)
flipper-active_record (0.25.4)
flipper (1.3.1)
concurrent-ruby (< 2)
flipper-active_record (1.3.1)
activerecord (>= 4.2, < 8)
flipper (~> 0.25.4)
flipper-ui (0.25.4)
flipper (~> 1.3.1)
flipper-ui (1.3.1)
erubi (>= 1.0.0, < 2.0.0)
flipper (~> 0.25.4)
rack (>= 1.4, < 3)
rack-protection (>= 1.5.3, <= 4.0.0)
flipper (~> 1.3.1)
rack (>= 1.4, < 4)
rack-protection (>= 1.5.3, < 5.0.0)
rack-session (>= 1.0.2, < 3.0.0)
sanitize (< 7)
formtastic (5.0.0)
actionpack (>= 6.0.0)
Expand Down Expand Up @@ -328,6 +330,8 @@ GEM
rack (~> 2.2, >= 2.2.4)
rack-proxy (0.7.7)
rack
rack-session (1.0.2)
rack (< 3)
rack-test (2.1.0)
rack (>= 1.3)
rails (6.1.7.10)
Expand Down Expand Up @@ -460,7 +464,7 @@ GEM
connection_pool (~> 2.2)
multi_json (~> 1.0)
thor (~> 1.1)
sanitize (6.1.0)
sanitize (6.1.3)
crass (~> 1.0.2)
nokogiri (>= 1.12.0)
selenium-webdriver (4.26.0)
Expand Down Expand Up @@ -585,9 +589,9 @@ DEPENDENCIES
delayed_job_active_record
exception_notification
factory_bot_rails
flipper (~> 0.25.0)
flipper-active_record (~> 0.25.0)
flipper-ui (~> 0.25.0)
flipper (~> 1.0)
flipper-active_record (~> 1.0)
flipper-ui (~> 1.0)
formtastic
json
jsonapi-resources!
Expand Down
107 changes: 57 additions & 50 deletions README.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -27,44 +27,43 @@ a organisation of 900 people.

<!-- toc -->

- [ Sequencescape](#-sequencescape)
- [Contents](#contents)
- [Documentation](#documentation)
- [Linting](#linting)
- [Requirements](#requirements)
- [Getting started (using Docker)](#getting-started-using-docker)
- [Getting started (using native installation)](#getting-started-using-native-installation)
- [Installing ruby](#installing-ruby)
- [rbenv](#rbenv)
- [Automatic Sequencescape setup](#automatic-sequencescape-setup)
- [Manual Sequencescape setup](#manual-sequencescape-setup)
- [Installing gems](#installing-gems)
- [Adjusting config](#adjusting-config)
- [Default setup](#default-setup)
- [Starting rails](#starting-rails)
- [Vite](#vite)
- [Delayed job](#delayed-job)
- [Message broker](#message-broker)
- [Testing](#testing)
- [Linting and formatting](#linting-and-formatting)
- [Rake tasks](#rake-tasks)
- [Supporting applications](#supporting-applications)
- [Barcode printing](#barcode-printing)
- [Plate barcode service](#plate-barcode-service)
- [Data warehousing](#data-warehousing)
- [Miscellaneous](#miscellaneous)
- [Lefthook](#lefthook)
- [Ruby warnings and rake 11](#ruby-warnings-and-rake-11)
- [NPG - Illumina tracking software](#npg---illumina-tracking-software)
- [Troubleshooting](#troubleshooting)
- [MySQL errors when installing](#mysql-errors-when-installing)
- [MySQL errors after system updates](#mysql-errors-after-system-updates)
- [Installing on Apple Silicon (M1)](#installing-on-apple-silicon-m1)
- [API V2 Authentication](#api-v2-authentication)
- [Publishing AMQP Messages](#publishing-amqp-messages)
- [Updating the table of contents](#updating-the-table-of-contents)
- [CI](#ci)
- [ERD](#erd)
- [Documentation](#documentation)
- [Linting](#linting)
- [Requirements](#requirements)
- [Getting started (using Docker)](#getting-started-using-docker)
- [Getting started (using native installation)](#getting-started-using-native-installation)
- [Installing ruby](#installing-ruby)
- [rbenv](#rbenv)
- [Automatic Sequencescape setup](#automatic-sequencescape-setup)
- [Manual Sequencescape setup](#manual-sequencescape-setup)
- [Installing gems](#installing-gems)
- [Adjusting config](#adjusting-config)
- [Default setup](#default-setup)
- [Starting rails](#starting-rails)
- [Vite](#vite)
- [Delayed job](#delayed-job)
- [Message broker](#message-broker)
- [Testing](#testing)
- [Linting and formatting](#linting-and-formatting)
- [Rake tasks](#rake-tasks)
- [Feature flags](#feature-flags)
- [Supporting applications](#supporting-applications)
- [Barcode printing](#barcode-printing)
- [Plate barcode service](#plate-barcode-service)
- [Data warehousing](#data-warehousing)
- [Miscellaneous](#miscellaneous)
- [Lefthook](#lefthook)
- [Ruby warnings and rake 11](#ruby-warnings-and-rake-11)
- [NPG - Illumina tracking software](#npg---illumina-tracking-software)
- [Troubleshooting](#troubleshooting)
- [MySQL errors when installing](#mysql-errors-when-installing)
- [MySQL errors after system updates](#mysql-errors-after-system-updates)
- [Installing on Apple Silicon (M1)](#installing-on-apple-silicon-m1)
- [API V2 Authentication](#api-v2-authentication)
- [Publishing AMQP Messages](#publishing-amqp-messages)
- [CI](#ci)
- [ERD](#erd)
- [Updating the table of contents](#updating-the-table-of-contents)

<!-- tocstop -->

Expand Down Expand Up @@ -343,6 +342,14 @@ of running standalone scripts multiple times.

A breakdown of the the available tasks and how to run them can be found [here](lib/tasks/README.md)

## Feature flags

The use of feature flags is encouraged for development.

[Flipper](https://github.com/flippercloud/flipper) is used for flag management and flags can be controlled from the `/flipper` route.

To create a new feature flag, update `config/feature_flags.yml`.

## Supporting applications

There are a number of services that are needed in certain parts of Sequencescape these are listed
Expand Down Expand Up @@ -491,22 +498,10 @@ After the first use, a cached file will be created in `data/avro_schema_cache` s
Because this is the first and only job doing this pubishing / RedPanda caching / Avro encoding, etc, there are parts which could be extracted in future if further jobs of this type are created.
This isn't necessary at this stage, but it seems wise to note the intended pattern of usage here for future work.

### Updating the table of contents

To update the table of contents after adding things to this README you can use the [markdown-toc](https://github.com/jonschlinkert/markdown-toc)
node module. To install it, make sure you have installed the dev dependencies from yarn. To update
the table of contents, run:

```shell
npx markdown-toc -i README.md --bullets "-"
```

### CI

The GH actions builds use the Knapsack-pro gem to reduce build time by parallelizing the RSpec and Cucumber tests. There is no need to regenerate the knapsack_rspec_report.json file, Knapsack Pro will dynamically allocate tests to ensure tests finish as close together as possible.

Copyright (c) 2007, 2010-2021 Genome Research Ltd.

### ERD

You can create a database entity relationship diagram, by specifying the title and attributes optionally, and view the output:
Expand All @@ -518,3 +513,15 @@ open erd.pdf
```

The command uses the [rails-erd](https://github.com/voormedia/rails-erd) gem.

### Updating the table of contents

To update the table of contents after adding things to this README you can use the [markdown-toc](https://github.com/jonschlinkert/markdown-toc) node module.
To install it, make sure you have installed the dev dependencies from yarn.
To update the table of contents, run:

```shell
npx markdown-toc -i README.md --bullets "-"
```

Copyright (c) 2007, 2010-2024 Genome Research Ltd.
2 changes: 1 addition & 1 deletion config/cucumber.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,6 @@ rerun = rerun.strip.gsub /\s/, ' '
rerun_opts = rerun.empty? ? "--format #{ENV['CUCUMBER_FORMAT'] || 'progress'} features" : "--format #{ENV['CUCUMBER_FORMAT'] || 'pretty'} #{rerun}"
std_opts = "--format #{ENV['CUCUMBER_FORMAT'] || 'pretty'} --strict --tags 'not @wip'"
%>
default: <%= std_opts %> features
default: <%= std_opts %> features --publish-quiet
wip: --tags @wip:3 --wip features
rerun: <%= rerun_opts %> --format rerun --out rerun.txt --strict --tags 'not @wip'
2 changes: 1 addition & 1 deletion config/routes.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -635,5 +635,5 @@
end
end

mount Flipper::UI.app(Flipper) => '/flipper', :constraints => user_is_admin
mount Flipper::UI.app => '/flipper', :constraints => user_is_admin
end
16 changes: 16 additions & 0 deletions db/migrate/20241106150235_change_flipper_gates_value_to_text.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,16 @@
# frozen_string_literal: true

class ChangeFlipperGatesValueToText < ActiveRecord::Migration[6.1]
def up
# Ensure this incremental update migration is idempotent
return unless connection.column_exists? :flipper_gates, :value, :string
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is this really needed? What would happen if the column were not already containing a string value, other than spending a bit longer than strictly needed doing the migration? If you do think it's needed, should it also be used in the down method?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Both this and the comment in the down method are very good points.
In this case the migration was automatically created by running bundle exec rails g flipper:active_record --force as requested by the upgraded version of the package, so I cannot take credit for this code.
But since it is making changes to our codebase I think you're right that it should meet some quality standards - such as being truely reversable.
Since I haven't suceeded in getting the cucumber tests to pass due to the migration - I'll give it a good once over.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Another alternative is to mark it as non-reversible as we've done in some previous migrations (I think I've seen it here anyway)


remove_index :flipper_gates, %i[feature_key key value] if index_exists? :flipper_gates, %i[feature_key key value]
change_column :flipper_gates, :value, :text
add_index :flipper_gates, %i[feature_key key value], unique: true, length: { value: 255 }
end

def down
change_column :flipper_gates, :value, :string
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Does this also need to do the index modifications to be a true reversal of the above?

end
end
6 changes: 3 additions & 3 deletions db/schema.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@
#
# It's strongly recommended that you check this file into your version control system.

ActiveRecord::Schema.define(version: 2024_11_06_103710) do
ActiveRecord::Schema.define(version: 2024_11_06_150235) do

create_table "aliquot_indices", id: :integer, charset: "utf8mb4", collation: "utf8mb4_unicode_ci", options: "ENGINE=InnoDB ROW_FORMAT=DYNAMIC", force: :cascade do |t|
t.integer "aliquot_id", null: false
Expand Down Expand Up @@ -488,10 +488,10 @@
create_table "flipper_gates", charset: "utf8mb4", collation: "utf8mb4_unicode_ci", force: :cascade do |t|
t.string "feature_key", null: false
t.string "key", null: false
t.string "value"
t.text "value"
t.datetime "created_at", precision: 6, null: false
t.datetime "updated_at", precision: 6, null: false
t.index ["feature_key", "key", "value"], name: "index_flipper_gates_on_feature_key_and_key_and_value", unique: true
t.index ["feature_key", "key", "value"], name: "index_flipper_gates_on_feature_key_and_key_and_value", unique: true, length: { value: 255 }
end

create_table "flowcell_types", charset: "utf8mb4", collation: "utf8mb4_unicode_ci", force: :cascade do |t|
Expand Down
Loading