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.

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

0.2.0 Introduce client Clone task #12

Merged
merged 6 commits into from
Aug 19, 2024
Merged

0.2.0 Introduce client Clone task #12

merged 6 commits into from
Aug 19, 2024

Conversation

sovetnik
Copy link
Owner

@sovetnik sovetnik commented Aug 3, 2024

No description provided.

@sovetnik sovetnik changed the title 0.2.0 Introduce Clone task 0.2.0 Introduce client Clone task Aug 4, 2024
Copy link
Collaborator

@am-kantox am-kantox left a comment

Choose a reason for hiding this comment

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

Mind to share the flaky error happening sometimes (when the race condition happens or like)?

I cannot reproduce it on my machine.

mix clone phase_id "token"

"""
|> Logger.warning()
Copy link
Collaborator

Choose a reason for hiding this comment

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

Mix.shell.info/1 makes it fancier, see this for an inspiration.

lib/mix/tasks/clone.ex Outdated Show resolved Hide resolved
lib/mix/tasks/clone.ex Show resolved Hide resolved
lib/mix/tasks/clone.ex Outdated Show resolved Hide resolved
defmodule Umwelt.Client.Clone do
@moduledoc "Clone main process"

use GenServer
Copy link
Collaborator

Choose a reason for hiding this comment

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

Suggested change
use GenServer
use GenServer, restart: :transient, shutdown: 10_000

Default for :restart is :permanent and I doubt restarting makes any sense here.

Copy link
Owner Author

@sovetnik sovetnik Aug 7, 2024

Choose a reason for hiding this comment

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

Looks like this change fixed both tests.
UPD: it wasn't .

alias Umwelt.Client

def start_link(module) do
Task.start_link(__MODULE__, :run, [module])
Copy link
Collaborator

Choose a reason for hiding this comment

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

Task.Supervisor would probably help to make sure all tasks are finished normally.

lib/umwelt/client/writer.ex Outdated Show resolved Hide resolved
lib/umwelt/client/writer.ex Outdated Show resolved Hide resolved
test/mix/tasks/clone_test.exs Outdated Show resolved Hide resolved
"Disco": "1",
"Disco.Chaos": "2",
"Disco.Discord": "3"
}}))
Copy link
Collaborator

Choose a reason for hiding this comment

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

I cannot believe formatter did not say anything here.

Copy link
Owner Author

Choose a reason for hiding this comment

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

But it did not.Maybe sigil ~s allows this kind of format?

@sovetnik
Copy link
Owner Author

sovetnik commented Aug 7, 2024

Mind to share the flaky error happening sometimes (when the race condition happens or like)

one:

 1) test run/1 writes new files correctly (Umwelt.Client.WriterTest)
     test/umwelt/client/writer_test.exs:25
     ** (exit) exited in: GenServer.call(Umwelt.Client.Agent, {:update, #Function<0.104241804/1 in Umwelt.Client.Agent.add_modules/1>}, 5000)
         ** (EXIT) no process: the process is not alive or there's no process currently associated with the given name, possibly because its application isn't started
     stacktrace:
       (elixir 1.17.2) lib/gen_server.ex:1121: GenServer.call/3
       test/umwelt/client/writer_test.exs:17: Umwelt.Client.WriterTest.__ex_unit_setup_0/1
       test/umwelt/client/writer_test.exs:1: Umwelt.Client.WriterTest.__ex_unit__/2

two:

  1) test run/1 writes new files correctly (Umwelt.Client.WriterTest)
     test/umwelt/client/writer_test.exs:25
     ** (ArgumentError) errors were found at the given arguments:

       * 1st argument: invalid destination

     code: log = capture_log(fn -> Writer.run(module) end)
     stacktrace:
       (erts 15.0.1) :erlang.send(Umwelt.Client.Clone, {:written, "Disco.Chaos"})
       (umwelt 0.2.0) lib/umwelt/client/writer.ex:17: Umwelt.Client.Writer.run/1
       (ex_unit 1.17.2) lib/ex_unit/capture_log.ex:113: ExUnit.CaptureLog.with_log/2
       (ex_unit 1.17.2) lib/ex_unit/capture_log.ex:75: ExUnit.CaptureLog.capture_log/2
       test/umwelt/client/writer_test.exs:29: (test)
       

@sovetnik
Copy link
Owner Author

sovetnik commented Aug 8, 2024

Captured failing test on CI!

@sovetnik sovetnik requested a review from am-kantox August 8, 2024 19:21
@sovetnik sovetnik force-pushed the client branch 2 times, most recently from 0d1fa08 to 4168e65 Compare August 18, 2024 06:38
@sovetnik
Copy link
Owner Author

It seems some timers fix the issue.

@sovetnik sovetnik merged commit 55f7711 into main Aug 19, 2024
6 checks passed
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.

2 participants