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

feat: Add docker compose file for notifier and worker #1138

Merged
merged 25 commits into from
Mar 8, 2023

Conversation

Terryhung
Copy link
Collaborator

@Terryhung Terryhung commented Feb 18, 2023

This PR supports user to deploy the notfier and worker independently.

Copy link
Contributor

@birdychang birdychang left a comment

Choose a reason for hiding this comment

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

Maybe use prod_ or something for file prefix instead of new_?

build/lily/new_notifier_config.toml Outdated Show resolved Hide resolved
build/lily/new_worker.env Outdated Show resolved Hide resolved
build/lily/new_worker_config.toml Outdated Show resolved Hide resolved
build/prometheus/new_prometheus.yml Outdated Show resolved Hide resolved
deployment/gce/docker-compose-notifier.yml Outdated Show resolved Hide resolved
deployment/gce/lily/worker_config.toml Outdated Show resolved Hide resolved
deployment/gce/lily/docker_init.sh Outdated Show resolved Hide resolved
Copy link
Contributor

@birdychang birdychang left a comment

Choose a reason for hiding this comment

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

deployment/gce/lily/worker_config.toml Outdated Show resolved Hide resolved
deployment/gce/lily/notifier.env Outdated Show resolved Hide resolved
deployment/gce/lily/worker.env Outdated Show resolved Hide resolved
deployment/gce/docker-compose-notifier.yml Outdated Show resolved Hide resolved
deployment/gce/docker-compose-worker.yml Outdated Show resolved Hide resolved
@birdychang
Copy link
Contributor

why another prometheus folder under prometheus?


[Pubsub]
Bootstrapper = false
RemoteTracer = "/dns4/pubsub-tracer.filecoin.io/tcp/4001/p2p/QmTd6UvR47vUidRNZ1ZKXHrAFhqTJAD27rKL9XYghEKgKX"
Copy link
Contributor

Choose a reason for hiding this comment

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

This should be configured in our instance either via an environment variable or adding directly on the config.toml and not part of the general deployment definition.

Copy link
Contributor

Choose a reason for hiding this comment

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

Copy link
Contributor

Choose a reason for hiding this comment

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

I think it should also be removed out of that config. It seems like a forgotten hard-coded value.

@Terryhung Terryhung marked this pull request as ready for review March 2, 2023 04:28
deployment/gce/lily/notifier.env Outdated Show resolved Hide resolved
deployment/gce/lily/worker.env Outdated Show resolved Hide resolved
@Terryhung
Copy link
Collaborator Author

why another prometheus folder under prometheus?

This prometheus has it's docker compose file and it's config.

Copy link
Contributor

@birdychang birdychang left a comment

Choose a reason for hiding this comment

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

LG! Thanks!

@Terryhung Terryhung changed the title [WIP] add notifier and worker docker compose feat: add notifier and worker docker compose Mar 8, 2023
@Terryhung Terryhung changed the title feat: add notifier and worker docker compose feat: Add docker compose for notifier and worker Mar 8, 2023
@Terryhung Terryhung changed the title feat: Add docker compose for notifier and worker feat: Add docker compose file for notifier and worker Mar 8, 2023
@birdychang birdychang merged commit 8250fa7 into master Mar 8, 2023
@birdychang birdychang deleted the terryhung/add-notifier-docker-compose branch March 8, 2023 16:52
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.

4 participants