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.

Sign up for GitHub

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

Jump to bottom

MF-708 - Assign Writer(s) to a channel #737

Merged
merged 5 commits into from
May 10, 2019
Merged

MF-708 - Assign Writer(s) to a channel #737

merged 5 commits into from
May 10, 2019

Conversation

anovakovic01
Copy link
Contributor

What does this do?

Adds the configuration file with the list of channels. Writers will only persist messages from specified channels.

Which issue(s) does this PR fix/relate to?

Resolves #708.

List any changes that modify/break current functionality

Configuration should be specified in order to persist messages from channels.

Have you included tests for your changes?

No.

Did you document any new/modified functionality?

Yes, I've updated README files.

Aleksandar Novakovic added 3 commits May 8, 2019 20:34
Add support for channel filtering using config
7d4ff03 
Add support for channel filtering using yaml configuration files
for writers.

Signed-off-by: Aleksandar Novakovic <aleksandar.novakovic@mainflux.com>
Update writers documentation with new env var
54f9b0c 
Signed-off-by: Aleksandar Novakovic <aleksandar.novakovic@mainflux.com>
Add info comment to configuration file
dd72124 
Signed-off-by: Aleksandar Novakovic <aleksandar.novakovic@mainflux.com>
@codecov-io
Copy link

codecov-io commented May 8, 2019 •
edited
Loading

Codecov Report

Merging #737 into master will decrease coverage by 0.2%.
The diff coverage is n/a.

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #737      +/-   ##
==========================================
- Coverage   85.29%   85.08%   -0.21%     
==========================================
  Files          66       66              
  Lines        4216     4232      +16     
==========================================
+ Hits         3596     3601       +5     
- Misses        416      430      +14     
+ Partials      204      201       -3
Impacted Files Coverage Δ
things/postgres/things.go 76.71% <0%> (-1.97%) ⬇️
things/postgres/channels.go 76.76% <0%> (-1.96%) ⬇️
things/service.go 90.79% <0%> (-1.23%) ⬇️
writers/cassandra/init.go 80% <0%> (+20%) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 886c8b2...ec375ae. Read the comment docs.

Fix configuration and update yaml dep
306dc75 
Signed-off-by: Aleksandar Novakovic <aleksandar.novakovic@mainflux.com>
drasko
drasko requested changes May 8, 2019
View reviewed changes
Copy link
Contributor

@drasko drasko left a comment

Choose a reason for hiding this comment

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

Please use TOML, as it is official config method of Mainflux. Also, please provide some documentation about the feature.

cmd/cassandra-writer/main.go Outdated
defPort = "8180"
defCluster = "127.0.0.1"
defKeyspace = "mainflux"
defChanCfgPath = "/config/channels.yaml"
Copy link
Contributor

Choose a reason for hiding this comment

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

We use TOML for all our configs in Mainflux.

cmd/influxdb-writer/main.go Outdated
@@ -41,6 +43,7 @@ const (
defDBPort = "8086"
defDBUser = "mainflux"
defDBPass = "mainflux"
defChanCfgPath = "/config/channels.yaml"
Copy link
Contributor

Choose a reason for hiding this comment

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

TOML

cmd/mongodb-writer/main.go Outdated
defDBName = "mainflux"
defDBHost = "localhost"
defDBPort = "27017"
defChanCfgPath = "/config/channels.yaml"
Copy link
Contributor

Choose a reason for hiding this comment

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

TOML

cmd/postgres-writer/main.go Outdated
@@ -42,6 +44,7 @@ const (
defDBSSLCert = ""
defDBSSLKey = ""
defDBSSLRootCert = ""
defChanCfgPath = "/config/channels.yaml"
Copy link
Contributor

Choose a reason for hiding this comment

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

TOML

@anovakovic01
Copy link
Contributor Author

@drasko I've updated README files and added doc comments in the configuration itself. I didn't know that we had any config files in Mainflux, especially TOML. Can you send me an example?

Update config from yaml to toml
ec375ae 
Signed-off-by: Aleksandar Novakovic <aleksandar.novakovic@mainflux.com>
drasko
drasko reviewed May 9, 2019
View reviewed changes
docker/addons/cassandra-writer/channels.toml
@@ -0,0 +1,4 @@
# If you want to listen on all channels, just pass one element ["*"], otherwise
# pass the list of channels.
[channels]
Copy link
Contributor

Choose a reason for hiding this comment

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

Why do we need a special table for this config? Would there be any other setting for channels except filter? I.e. would just

channels = [...]

do the job?

Moreover, if all that we need is alist of channels, maybe we do not need even TOML? Maybe a simplle file with one channel ID per line is enough?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

We talked about adding support for configuration for every service (instead of using current env vars). That's why I added it as a separate table (this config will grow, and in the end, this config will create a lot of other fields). We can use simple file, but I would keep standard config format (especially because this config will grow).

Copy link
Contributor

Choose a reason for hiding this comment

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

Agreed.

drasko
drasko approved these changes May 9, 2019
View reviewed changes
Copy link
Contributor

@drasko drasko left a comment

Choose a reason for hiding this comment

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

LGTM

drasko
drasko approved these changes May 9, 2019
View reviewed changes
Copy link
Contributor

@drasko drasko left a comment

Choose a reason for hiding this comment

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

LGTM

@nmarcetic nmarcetic merged commit 5e5fe88 into absmach:master May 10, 2019
dborovcanin pushed a commit to dborovcanin/magistrala that referenced this pull request May 13, 2019
@dborovcanin
MF-708 - Assign Writer(s) to a channel (absmach#737)
066e2b4 
* Add support for channel filtering using config

Add support for channel filtering using yaml configuration files
for writers.

Signed-off-by: Aleksandar Novakovic <aleksandar.novakovic@mainflux.com>

* Update writers documentation with new env var

Signed-off-by: Aleksandar Novakovic <aleksandar.novakovic@mainflux.com>

* Add info comment to configuration file

Signed-off-by: Aleksandar Novakovic <aleksandar.novakovic@mainflux.com>

* Fix configuration and update yaml dep

Signed-off-by: Aleksandar Novakovic <aleksandar.novakovic@mainflux.com>

* Update config from yaml to toml

Signed-off-by: Aleksandar Novakovic <aleksandar.novakovic@mainflux.com>
dborovcanin pushed a commit to dborovcanin/magistrala that referenced this pull request May 13, 2019
@dborovcanin
MF-708 - Assign Writer(s) to a channel (absmach#737)
c7b4dbf 
* Add support for channel filtering using config

Add support for channel filtering using yaml configuration files
for writers.

Signed-off-by: Aleksandar Novakovic <aleksandar.novakovic@mainflux.com>

* Update writers documentation with new env var

Signed-off-by: Aleksandar Novakovic <aleksandar.novakovic@mainflux.com>

* Add info comment to configuration file

Signed-off-by: Aleksandar Novakovic <aleksandar.novakovic@mainflux.com>

* Fix configuration and update yaml dep

Signed-off-by: Aleksandar Novakovic <aleksandar.novakovic@mainflux.com>

* Update config from yaml to toml

Signed-off-by: Aleksandar Novakovic <aleksandar.novakovic@mainflux.com>
davide83 pushed a commit to davide83/mainflux that referenced this pull request May 13, 2019
@davide83
MF-708 - Assign Writer(s) to a channel (absmach#737)
46f7300 
* Add support for channel filtering using config

Add support for channel filtering using yaml configuration files
for writers.

Signed-off-by: Aleksandar Novakovic <aleksandar.novakovic@mainflux.com>

* Update writers documentation with new env var

Signed-off-by: Aleksandar Novakovic <aleksandar.novakovic@mainflux.com>

* Add info comment to configuration file

Signed-off-by: Aleksandar Novakovic <aleksandar.novakovic@mainflux.com>

* Fix configuration and update yaml dep

Signed-off-by: Aleksandar Novakovic <aleksandar.novakovic@mainflux.com>

* Update config from yaml to toml

Signed-off-by: Aleksandar Novakovic <aleksandar.novakovic@mainflux.com>
rugwirobaker pushed a commit to rugwirobaker/mainflux that referenced this pull request Jun 26, 2019
@rugwirobaker
MF-708 - Assign Writer(s) to a channel (absmach#737)
a00a235 
* Add support for channel filtering using config

Add support for channel filtering using yaml configuration files
for writers.

Signed-off-by: Aleksandar Novakovic <aleksandar.novakovic@mainflux.com>

* Update writers documentation with new env var

Signed-off-by: Aleksandar Novakovic <aleksandar.novakovic@mainflux.com>

* Add info comment to configuration file

Signed-off-by: Aleksandar Novakovic <aleksandar.novakovic@mainflux.com>

* Fix configuration and update yaml dep

Signed-off-by: Aleksandar Novakovic <aleksandar.novakovic@mainflux.com>

* Update config from yaml to toml

Signed-off-by: Aleksandar Novakovic <aleksandar.novakovic@mainflux.com>
@nmarcetic nmarcetic mentioned this pull request Jul 10, 2019
Closed
manuio pushed a commit that referenced this pull request Oct 12, 2020
@nmarcetic
MF-708 - Assign Writer(s) to a channel (#737)
ed754c6 
* Add support for channel filtering using config

Add support for channel filtering using yaml configuration files
for writers.

Signed-off-by: Aleksandar Novakovic <aleksandar.novakovic@mainflux.com>

* Update writers documentation with new env var

Signed-off-by: Aleksandar Novakovic <aleksandar.novakovic@mainflux.com>

* Add info comment to configuration file

Signed-off-by: Aleksandar Novakovic <aleksandar.novakovic@mainflux.com>

* Fix configuration and update yaml dep

Signed-off-by: Aleksandar Novakovic <aleksandar.novakovic@mainflux.com>

* Update config from yaml to toml

Signed-off-by: Aleksandar Novakovic <aleksandar.novakovic@mainflux.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@nmarcetic nmarcetic nmarcetic approved these changes

@drasko drasko drasko approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Assign Writer(s) to a channel
4 participants
@anovakovic01 @codecov-io @drasko @nmarcetic