Skip to content

Conversation

@mpeck
Copy link
Contributor

@mpeck mpeck commented Oct 28, 2016

This adds/updates a number of error messages to make it easier for users to troubleshoot issues.

resolves #1092

  • Executing a command in a bundle not assigned to a relay group
    • Bundle 'test' is not assigned to a relay group. Assign the bundle to a relay group and try again.
  • Executing a command that only exists on a disabled relay
    • No Cog Relays supporting thetestbundle are currently available. There are one or more relays serving the bundle, but they are disabled. Enable an appropriate relay and try again.
  • Updates to the standard "No Cog Relays supporting..." error
    • No Cog Relays supporting thetestbundle are currently online. If you just assigned the bundle to a relay group you will need to wait for the relay refresh interval to pick up the new bundle.

Provided by piper:

  • Executing a command in a disabled bundle
    • Bundle 'mybundle' is disabled. Please enable it and try running the command again.
  • Executing a command that does not exist in a bundle
    • Bundle '#{bundle}' doesn't contain a command named '#{text}'.

@mpeck mpeck added the review label Oct 28, 2016
@mpeck mpeck force-pushed the peck/error-messages branch 2 times, most recently from 76fa824 to ad7bfb1 Compare October 28, 2016 17:17
@mpeck mpeck changed the title error messages error messages WIP Oct 28, 2016
@mpeck mpeck changed the title error messages WIP error messages Oct 30, 2016
relays = Map.delete(relays, bundle_name)
assign_relay(relays, bundle_name, bundle_version)
end
# If the bundle is assigned to a relay group carry on, otherwise bail
Copy link
Member

Choose a reason for hiding this comment

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

Flipping the logic will reduce the number of DB queries issued to execute a pipeline:

case Map.get(relays, bundle_name) do
  # No assignment so let's pick one
  nil ->
    case Relays.pick_one(bundle_name, bundle_version) do
      # No relays available
      {:error, :no_relays} ->
        {nil, relays}
        # relays exist, but are disabled
      {:error, :no_enabled_relays} ->
        # Query database to clarify error condition before reporting to user
        if Cog.Repository.Bundles.assigned_to_group?(bundle_name) do
          {:no_enabled_relays, relays}
        else
          {:no_relay_group, relays}
        end
      # Store the selected relay in the relay cache
      {:ok, relay} ->
        {relay, Map.put(relays, bundle_name, relay)}
    end
    # Relay was previously assigned
  relay ->
  # Is the bundle still available on the relay? If not, remove the current assignment from the cache
  # and select a new relay
    if Relays.relay_available?(relay, bundle_name, bundle_version) do
      {relay, relays}
    else
      relays = Map.delete(relays, bundle_name)
      assign_relay(relays, bundle_name, bundle_version)
    end
end

relays = Map.get(tracker.map, {bundle_name, bundle_version}, MapSet.new)
enabled_relays = MapSet.difference(relays, tracker.disabled)

{MapSet.to_list(relays), MapSet.to_list(enabled_relays)}
Copy link
Member

Choose a reason for hiding this comment

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

I think returning descriptive atoms instead of sets of relays would be more intention revealing.

{[], []} becomes :no_relays
{[...], []} becomes :no_enabled_relays
{[...], [...]} becomes [...]

where [...] indicates a non-empty list.

@mpeck mpeck force-pushed the peck/error-messages branch 2 times, most recently from 9ffe784 to 33b3681 Compare October 31, 2016 16:33
@spec is_bundle_available?(t, relay_id, bundle_name, version) :: boolean()
def is_bundle_available?(tracker, relay, bundle_name, bundle_version) do
available_relays = relays(tracker, bundle_name, bundle_version)
{_all_relays, available_relays} = relays(tracker, bundle_name, bundle_version)
Copy link
Member

Choose a reason for hiding this comment

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

Does this work if relays/3 returns one of the error tuples?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

nope, that's a bug. Missed that bit from the previous function return.

@mpeck mpeck force-pushed the peck/error-messages branch from 68a5b82 to 24397bb Compare October 31, 2016 18:02
@mpeck mpeck merged commit 24397bb into master Oct 31, 2016
@mpeck mpeck removed the review label Oct 31, 2016
@mpeck mpeck deleted the peck/error-messages branch October 31, 2016 18:19
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Improve "command not found in any bundle" error message

3 participants