Short notes from engineering

then in Ruby feels like a little sister to the as-> macro from Clojure.

Most of the API code (and several other parts) in Tramline make heavy use of then.

example, example, example.

There's a couple of interesting things of note here.

One, I think numbered params have a lot of scope in removing some of the block cruft and overnaming of things across then blocks. Especially if the next in chain is visually obvious.

@client.workflows(repo)
       .then { fetch_workflows(_1) }
       .then { pick_active(_1) }
       .then { transform_keys(_1) }

The above feels more natural and less noisy than naming each intermediate step with similar sounding variable names.

The second one is a controversial (perhaps even wrong) point. It seems to me that since then is just a function, the general debuggability of something going wrong in the pipeline is easier to find out in the chain.

My (now fading) experience with threading macro debuggability in Clojure has been less efficient; I end up adding taps and spies and macroexpands to figure out what part of the chain broke.

I prefer an experience like:

1. write a pipeline
2. pipeline breaks in expression number 2
3. error output nudges you in the direction of just tweaking expression number 2
4. make the fix and the pipeline starts working again

All these years of using Rails, I had no idea has_one did this.

More importantly with a relationship like follows,

class Model < ActiveRecord
  has_one :build
end

class Build < ActiveRecord
  has_one_attached_file :file
end

If I create_build on Model if one already exists, it will:

1. Delete that Build, create a new one and attach it to Model
2. Kickoff an ActiveStorage::PurgeJob and delete the file from GCS/AWS

This is highly uncomfortable.

I have been using return a fair bit in transactions in Rails. In 7, these rollback the transaction and I use them as so.

They obviously require very careful writing and to remember that the return actually just doesn't jump out of the block, it cancels the transaction.

I prefer the fact that return from 8 will throw an exception, but I've found that using Github::Result around transactions as a pattern comes pretty close or is better (in some cases) already.

Consider this example code that creates and merges a pull request,

def create_and_merge!
  return Result.new(ok?: false) unless create.ok?
  upserted_pull_request =
    @new_pull_request.update_or_insert!(create.value)

  transaction do
    upserted_pull_request.close! # close the PR

    if merge.ok?
      Result.new(ok?: true)
    else
      return Result.new(ok?: false, error: "Failed!")
    end
  end
end

def create
  # creates a PR and returns a Result
end

def merge
  # merges a PR and returns a Result
end

Result = Struct.new(:ok?, :error, :value, keyword_init: true)

One problem here is that we're using custom Result objects which are not very chainable. But the other more shape-y problem is that we're having to check the output from merge, return an ok-Result or else cancel the transaction and then return a not-ok-Result. This not only feels like excessive work but also the use of return is unfortunate to essentially carry out a rollback + value type scenario.

With Github::Result we can rewrite it much more cleanly,

def create_and_merge!
   return GitHub::Result.new { raise CreateError } unless create.ok?
   upserted_pull_request =
     @new_pull_request.update_or_insert!(create.value!)

   GitHub::Result.new do
     transaction do
       upserted_pull_request.close! # close the PR
       merge.value!
     end
  end
end

As long as merge throws an exception, value! will raise it, rollback (throwing an exception will rollback) and opaquely pass it further up to the wrappper Result. This allows us to avoid return magic and ugly raises in the middle of the transaction block and chain the exceptions up.

This is terrible.

I decided to add stricter linting to the codebase. Since I like what standard offers, I simply extended it with rails specific lints, in the following way:

require:
  - standard
  - rubocop-rails
  - rubocop-rspec
  - rubocop-performance

inherit_gem:
  standard: config/ruby-3.0.yml

AllCops:
  TargetRubyVersion: 3.1
  NewCops: enable
  Exclude:
    - bin/**/*
    - public/**/*
    - vendor/**/*
    - db/schema.rb

Rails:
  Enabled: true

RSpec:
  Enabled: true

RSpec/ExampleLength:
  Enabled: false

RSpec/MultipleExpectations:
  Max: 4

Performance:
  Enabled: true

This ensures standard is used for ruby things, and rails, rspec and some other performance related checks in addition to it via rubocop.

I came across this suggestion while fixing the 100 odd offenses it threw up,

RSpec/NamedSubject:
  Name your test subject if you need to reference it explicitly.

The fix,

subject(:run) { create(:releases_train_run) }

But what even is this? Isn't this just a let really? You can't really use the is_expected.to shortcut either since it's now named. I cannot possibly make the claim that this reads better.

I think it's just one of those annoying rspec maximalisms.

Here's a spectrum of componentizing views I've been thinking about lately (in Rails):

1. Extract logical helpers and partials

They are often like stuffing away code inside a Module and calling it a refactor. But it is cheap and effective enough to start off with. You often run into "too many allocations" issues, with nested views and partials. They also don't abstract state very well. Arguments for their limitations have been made ad naseam - ex.

Completed 200 OK in 86ms (Views: 83.6ms | ActiveRecord: 1.3ms | Allocations: 29347)

2. Presenter objects / ViewComponents

Solid ideas here. Fair bit of upfront work required and you have to be meticulous about the granularity of your components. These claim to be very fast by precompiling templates at rails boot.

3. SPAs, building JSX components (or whatever else is out there now)

I'm not convinced I'd need to do this. I think Stimulus / Hotwired is powerful enough for all our needs for the forseeable future (famous last words). I still remember the joy of ripping out all the react from a previous codebase I worked on and rewrote them in a much simpler, basic markup and js.

#1 ➡ #2 hopefuly in a few months time.

When I originally kicked things off, I added a lot of big transactions around various api-call-surrounding-database-ops. This was knowingly a cheap hack to avoid bad state changes and delaying the inevitable: careful and tedious handling of errors and control-flow.

I'm now slowly refactoring things to unpack these big transactions and I realize many of them are actually unnecessary. I'm employing yet another cheap Result object to signify boundary-states,

Result = Struct.new(:ok?, :error, :value, keyword_init: true)

def good_op
  Result.new(ok?: true, value: 1)
end

def bad_op
  Result.new(ok?: false, error: "Did not work")
end

operation.ok?
operation.value
operation.error

This is a little bit silly, because it requires you to ensure that value is only present alongside the ok? and the others are falsey in the error case. But it is very simple and quick to get a refactor busted out.

I have used Github::Result in the past, but recently discovered dry-rb -- many of these look very sensible and immediately useful. Hopefully the next step!

There are a few tight, but rare-enough race-conditions lingering about and they are constantly bothering me. They just exist as FIXMEs in the code atm.

Most of the these seem to be solvable by pessimistic row-locks, but I'm already dreading the littering of necessary but unintuitive placements of various row-locks throughout the code-base.

Would modelling this as a pure declarative orchestrator help keep this in check?

Not sure.

The views currently are a hot mess, giant nested ERB templates. I'm not convinced of jumping into ViewComponents yet, since the shape of this UI is so nebulous and will definitely change dramatically and I do not want to deal with more objects and more abstractions and more code at the moment.

I'll deal with the complex markup like it's 1999.

It's not terribly hard to come up with a state-machine DSL like aasm by hand, but the ActiveRecord support is very handy.

I'm using it with an enum against a status field with the following options:

{
  column: :status,
  requires_lock: true,
  requires_new_transaction: false,
  enum: true,
  create_scopes: false
}

The requires_new_transaction is false to avoid landing into unnecessary nested transactions. I'll keep my transactions explicit and intentional. The enum allows me to play well with the Rails enum which already generates scopes, so I keep create_scopes off.

The requires_lock is seemingly quite handy at first glance, since I require locking rows a fair bit because of the distributed nature of the app. But the level at which it locks is far too granular. I generally seem to require locking rows for far longer and more work than simply transitioning state and not everything can be sanely encapsulated in a before or after. I'd rather provide the library with methods that cause a state transition than the inverted way it currently operates: generating methods to transition for you and then locking within that event.

I'm finding keeping POROs seperated away from models/ a mental relief for some reason. I think I'm going to stick to it.

The log streams feature in Render is a little bit strange. From the docs,

Render Log Streams forward logs from your web services,
private services, background workers, databases,
and cron jobs to any logging provider that supplies a TLS-enabled syslog drain.

I hooked this up with datadog and it is indeed a syslog drain across all services. This means my newly setup datadog is now filled with thousands of non-application related logs. Not only now do I have to filter all these out, but also pay for them.

Surely a log streaming pipeline should have a minimum of a service-level (if not content-level) filtering as baseline features?