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

Various bin script fixes #148

Merged
merged 12 commits into from
Feb 6, 2018
Merged

Various bin script fixes #148

merged 12 commits into from
Feb 6, 2018

Conversation

chombium
Copy link
Collaborator

This pull request fixes #147

Best Regards,
Jovan

@chombium chombium requested a review from a team as a code owner January 27, 2018 02:05
For Cassandra follow the instructions at http://cassandra.apache.org/download/

After installing Cassandra you should create the two keypspaces that Mainflux uses:
Copy link
Contributor

Choose a reason for hiding this comment

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

Fix typo in word keyspace.


After installing Cassandra you should create the two keypspaces that Mainflux uses:

cqlsh> CREATE KEYSPACE IF NOT EXISTS manager WITH REPLICATION = { 'class' : 'SimpleStrategy', 'replication_factor' : 1 };
Copy link
Contributor

Choose a reason for hiding this comment

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

Please note that this replication factor is acceptable either for "quick-and-dirty" demo deployments, or for constrained environments. This should be noted in the docs as well.

@@ -17,8 +17,10 @@ mkdir -p $GOBIN
# Mainflux Go microservices
go get -d -v github.com/mainflux/mainflux
cd $GOPATH/src/github.com/mainflux/mainflux
make clean
Copy link
Contributor

Choose a reason for hiding this comment

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

I would propose to issue clean only if it is needed - i.e. if build exists

make
make install
make clean
Copy link
Contributor

Choose a reason for hiding this comment

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

No need to clean artifacts IMHO. Existing build gives indication to developer that something actually happened (compilation, etc...). @mijicd opinion?

@chombium
Copy link
Collaborator Author

Hi,

@drasko You are right. We don't need the cleaning, I have removed it.

@mijicd You are right, the creation of the keyspaces depends on the way how Cassandra is installed and configured. I was wandering how to create the keyspaces and I found the same commands in bin/mainflux-docker.sh, so I copied them in the bin/mainflux-install. Now I've added a short description on what the parameters of the "create keyspaces" command do and pointed out to check the Cassandra docs for details. I hope that's enough

Best Regards,
Jovan

@@ -21,10 +21,7 @@ sleep 0.1
mainflux-http &
mainflux-manager &
mainflux-writer &
mainflux-coap &
Copy link
Contributor

Choose a reason for hiding this comment

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

Why removing CoAP server? It is experimental, but still useful for many scenarios.

mainflux-launch.sh script is not a solution for professional deployment. It is for dev purposes also. In that case - let's keep CoAP server, and if developer does not need it - he needs to comment out this line.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

thanks for the tip @drasko
I was wandering why it was removed from the Makefile with #105 and it was still there in the launch script, so I thought we should keep the build and install script in sync.

I've added it back the launching of the CoAP server in the script. To make the things a bit clearer, I've also added small explanation how to build the CoAP server.

The explanation is also added to the docs with https://github.com/mainflux/doc/pull/13

Best Regards,
Jovan

README.md Outdated
@@ -54,6 +54,16 @@ To download all the sources, and place them in appropriate locations (i.e. $GOPA
[installation script](bin/mainflux-install.sh). Once it completes, the script will provide the
instructions on how to finish the manual installation (i.e. install the required infrastructure).

Please note that the CoAP server is not installed with the installation script, but the CoAP server
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 would be better to add CoAP build to Makefile. CoAP should become a part of mainflux-1.0 release (even though this wil remain experimental).

Signed-off-by: Jovan Kostovski <chombium@gmail.com>
Signed-off-by: Jovan Kostovski <chombium@gmail.com>
Signed-off-by: Jovan Kostovski <chombium@gmail.com>

Added a .gitignore file with the build folder in it, so that
if someone bulds Mainflux localy the build folder does not end
up in the repo.
Signed-off-by: Jovan Kostovski <chombium@gmail.com>

bugfix for the case where the script was started without an argument.
The help was printed, but the _mainflux_docker was called as well, which
was causing the following error:
./mainflux-docker.sh: line 163: $1: unbound variable
Signed-off-by: Jovan Kostovski <chombium@gmail.com>
Signed-off-by: Jovan Kostovski <chombium@gmail.com>
Signed-off-by: Jovan Kostovski <chombium@gmail.com>

In the README.md it is written that the install script will guide the user how to setup the infrastructure
for running Mainflux, but there was a missing description on which Cassandra keyspacess are needed and how to create them.
Signed-off-by: Jovan Kostovski <chombium@gmail.com>
Signed-off-by: Jovan Kostovski <chombium@gmail.com>
Signed-off-by: Jovan Kostovski <chombium@gmail.com>
Signed-off-by: Jovan Kostovski <chombium@gmail.com>

Signed-off-by: Jovan Kostovski <chombium@gmail.com>
Signed-off-by: Jovan Kostovski <chombium@gmail.com>

Signed-off-by: Jovan Kostovski <chombium@gmail.com>
@chombium
Copy link
Collaborator Author

chombium commented Feb 6, 2018

I've added the CoAP Server back in the Makefile and added it to the docker compose as discussed in #153

@drasko
Copy link
Contributor

drasko commented Feb 6, 2018

LGTM

@drasko drasko merged commit f849588 into absmach:master Feb 6, 2018
@drasko
Copy link
Contributor

drasko commented Feb 6, 2018

Merged, thanks!

@chombium chombium deleted the mainflux-147 branch February 6, 2018 14:53
manuio pushed a commit that referenced this pull request Oct 12, 2020
arvindh123 pushed a commit to arvindh123/magistrala that referenced this pull request Dec 17, 2023
* Refactor invitation to list invitations by state

Add states when fetching invitations that is:
- pending
- accepted
- all
- unknown

The changes focus on refactoring the functions responsible for retrieving and updating invitation data from a PostgreSQL database. Additionally, improvements have been made to the error handling mechanism. The commit also introduces new test cases to verify the functionality of the implemented functions.

Signed-off-by: Rodney Osodo <28790446+rodneyosodo@users.noreply.github.com>

* Rename `todbInviation` to `toDBInvitation`

This is to align with the rest of the codebase

Signed-off-by: Rodney Osodo <28790446+rodneyosodo@users.noreply.github.com>

* Refactor invitation state constants and string conversions

- Refactor the constants for invitation states in the state.go file.
- Rename the "pendingState" constant to "pending" and the "acceptedState" constant to "accepted".
- Update the corresponding String() function to return the new constant names.
- Update the ToState() function to handle the new constant names.

This commit improves the clarity and consistency of the invitation state constants and their string representations.

Signed-off-by: Rodney Osodo <28790446+rodneyosodo@users.noreply.github.com>

---------

Signed-off-by: Rodney Osodo <28790446+rodneyosodo@users.noreply.github.com>
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.

Various small bugs in the bin scripts
3 participants