Skip to content

Created a docker-compose file for development purpose#1644

Merged
angelo-v merged 3 commits into
nodeSolidServer:mainfrom
paulorodriguesxv:docker-compose
Jan 21, 2022
Merged

Created a docker-compose file for development purpose#1644
angelo-v merged 3 commits into
nodeSolidServer:mainfrom
paulorodriguesxv:docker-compose

Conversation

@paulorodriguesxv
Copy link
Copy Markdown
Contributor

It was created a docker-compose file for development purpose. It's facilitate server execution with no necessity of using solid-test script

…cilitate server execution with no necessity of using solid-test script
Copy link
Copy Markdown
Member

@TallTed TallTed left a comment

Choose a reason for hiding this comment

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

One additional tweak should be made to line 257 of README.md,
changing the existing —

Run with:

— to —

Once the image is built by either method, run it with —

Comment thread README.md Outdated
Comment on lines +240 to +242
If you want to use Docker in development, then you can build it locally with:

Using docker-compose
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Suggested change
If you want to use Docker in development, then you can build it locally with:
Using docker-compose
If you want to use Docker in development, you can build the image
locally with either docker-compose —

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Hi @TallTed.

Text adjusted. Thanks!

Comment thread README.md Outdated
docker-compose up -d
```

or build image manually and run it:
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Suggested change
or build image manually and run it:
or these manual commands —

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Hi @TallTed.

Text adjusted. Thanks!

Comment thread README.md Outdated
### Development usage

If you want to use Docker in development, then you can build it locally with:
If you want to use Docker in development, you can build and run the image locally with either docker-compose:
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Colon is not correct here. M-dash is.

Suggested change
If you want to use Docker in development, you can build and run the image locally with either docker-compose:
If you want to use Docker in development, you can build and run the image locally with either docker-compose

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Ok. I replaced colon by M-dash. Please, check it's ok now. Tks.

Comment thread README.md Outdated
```

or build image manually and run it:
or these manual commands:
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

M-dash leading into continued text is correct. Closing colon is also not correct; M-dash is again.

Suggested change
or these manual commands:
or these manual commands

Copy link
Copy Markdown
Member

@TallTed TallTed left a comment

Choose a reason for hiding this comment

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

Re-suggesting a couple of changes.

It appears to me that the docker-compose instruction does not launch the image. If that is correct, then either the image launch following the manual build should be broken back out (so it applies to both build methods), or an image launch command should be added to the docker-compose command sequence (so both build sequences end with image launch).

@paulorodriguesxv
Copy link
Copy Markdown
Contributor Author

Re-suggesting a couple of changes.

It appears to me that the docker-compose instruction does not launch the image. If that is correct, then either the image launch following the manual build should be broken back out (so it applies to both build methods), or an image launch command should be added to the docker-compose command sequence (so both build sequences end with image launch).

Hi @TallTed

docker-compose comand here, in the way it was created, build the image and launch them. Once I'm doing docker-compose "up -d", it's launch the image. You could observe at the sscrip that there is a<build .> key, it's mean that docker-compose will build the image in the case it wasn't built yet

Comment thread docker-compose.yml
container_name: solid
ports:
- "8443:8443"
entrypoint: npm run solid start -- --no-reject-unauthorized
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

question Why is --no-reject-unauthorized added? And why is the same(?) thing added as an env variable below as well?

Copy link
Copy Markdown
Contributor Author

@paulorodriguesxv paulorodriguesxv Jan 20, 2022

Choose a reason for hiding this comment

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

Hi @angelo-v

Because it's exactly same thing that bin/solid-test is doing, as you can see:

// Disable rejectUnauthorized when starting the server
if [ "$COMMAND" == "start" ]; then
ADD_FLAGS="--no-reject-unauthorized"
export NODE_TLS_REJECT_UNAUTHORIZED=0
fi

Respectfully speaking and excuse my sincerity, but this is simple docker-compose to help developer (like me) to running up application easily. But it is stuck in here for a long 1 month and ithas been generating so much questions that seems unbelieve to me.. Please feel free to decline.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Thanks for the hint @paulorodriguesxv I now understand and I think we can just merge it.

I totally understand your frustration. Unfortunately node-solid-server does not get much attention from the community anymore. Only a tiny group of volunteers keeping an eye on it more or less. I have the permission to merge and will do it, but I am not a NSS maintainer in any regard

@angelo-v
Copy link
Copy Markdown
Contributor

angelo-v commented Jan 21, 2022

The build failed at "Run bash test/surface/run-solid-test-suite.sh $BRANCH_NAME" because this does not work well with forks. Since the rest of the pipeline ran successfully and the solid-test-suite is not relevant here, I wil merge anyway

@angelo-v angelo-v merged commit 1dbee5e into nodeSolidServer:main Jan 21, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants