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

Conversion to umbrella project and other improvements #31

Merged
merged 33 commits into from
Jan 23, 2018
Merged

Conversation

kevinbader
Copy link
Contributor

No description provided.

kevinbader and others added 25 commits December 14, 2017 23:15
For a reliable startup, sup_wrapper is needed after all..
@kevinbader kevinbader added this to the 2.0.0 milestone Jan 21, 2018
@kevinbader
Copy link
Contributor Author

@mmacai with 1.1.0 out the door, it's time to get this merged into mainline and iterate from there

Copy link
Collaborator

@mmacai mmacai left a comment

Choose a reason for hiding this comment

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

mix docs outputs:

warning: function UUID.uuid4/0 is undefined (module UUID is not available)
  lib/rig_inbound_gateway/request_logger/kafka.ex:34

same message produced by running mix phx.server

docker build ... failed with:

** (File.Error) could not open "./guides/operator-guide.md": no such file or directory
    (elixir) lib/file.ex:1183: File.open!/2

@@ -0,0 +1,20 @@
# RigApi
Copy link
Collaborator

Choose a reason for hiding this comment

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

Do we want to keep this file? or maybe better question do we want to have README.md files for each umbrella app?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I'll keep the README files but only include a link to the developer's guide

```elixir
def deps do
[
{:rig_auth, "~> 0.1.0"}
Copy link
Collaborator

Choose a reason for hiding this comment

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

Should be 2.0.0, probably.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I'd say we remove this altogether

tracker =
Stubr.stub!(
[
# @callback track(jti: String.t, expiry: Timex.DateTime.t) :: {:ok, String.t}
Copy link
Collaborator

Choose a reason for hiding this comment

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

Can be removed? Same for commented callbacks below.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

hm that's meant as documentation as to what the mock should provide. But it's not checked by the compiler, so perhaps should be removed, not sure

watchers: []

# Do not include metadata nor timestamps in development logs TODO why?
#config :logger, :console, format: "[$level] $message\n"
Copy link
Collaborator

Choose a reason for hiding this comment

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

Remove?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

jep

@@ -77,6 +77,20 @@ defmodule RigWeb.Presence.Channel do
end
end

@doc """
Sends off outgoing messages.
Copy link
Collaborator

Choose a reason for hiding this comment

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

Didn't know about this one.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I still think it's rather strange


Work in progress:

- HTTP API
Copy link
Collaborator

Choose a reason for hiding this comment

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

Isn't this implemented or is the message API endpoint something different?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

removed that

{:brod, "~> 3.3"},
# JSON parser:
{:poison, "~> 2.0 or ~> 3.0"},
# HTTP server:
Copy link
Collaborator

Choose a reason for hiding this comment

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

Remove deps?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

done

:topic,
:partition,
:offset,
# aws_sqs_metadata
Copy link
Collaborator

Choose a reason for hiding this comment

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

I guess this shouldn't be here yet.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

true

config/dev.exs Outdated
watchers: []

# Do not include metadata nor timestamps in development logs TODO why?
# Do not include metadata nor timestamps in development logs
#config :logger, :console, format: "[$level] $message\n"
Copy link
Collaborator

Choose a reason for hiding this comment

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

Remove?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

done

version Outdated
@@ -0,0 +1,4 @@
%{
rig: "2.0.0-dev",
elixir: "~> 1.5",
Copy link
Collaborator

Choose a reason for hiding this comment

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

We should be able finally use 1.6. Same goes for Dockerfile.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

okay

@mmacai
Copy link
Collaborator

mmacai commented Jan 22, 2018

  • Missing COPY apps/rig_auth/config /opt/sites/rig/apps/rig_auth/config => leads to error during docker build ...

  • Since RUN mix release.init step was removed from Dockerfile, it should be removed from .gitignore and run manually MIX_ENV=prod mix release.init

  • even the fixes above, still getting error at the end of docker build ...

==> Assembling release..
==> Building release reactive_interaction_gateway:0.1.0 using environment prod
==> Including ERTS 9.2 from /usr/local/lib/erlang/erts-9.2
==> Packaging release..
==> Release packaging failed due to errors:
    Cannot add file sys.config to tar file - [{error,{25,erl_parse,["syntax error before: ",["Fun"]]}}]

@mmacai
Copy link
Collaborator

mmacai commented Jan 23, 2018

Tested with channels example and with some minor changes in example worked for both ws and sse.

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.

None yet

2 participants