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

Update 2019-01-01-00_overview.md #495

Closed
wants to merge 1 commit into from
Closed

Update 2019-01-01-00_overview.md #495

wants to merge 1 commit into from

Conversation

xpat
Copy link

@xpat xpat commented Jan 14, 2021

I suggest two small changes:

The first, to keep things consistent, was to delete the word port. If the word port remains, then the example should be https://your.host.example.org:8443

The second, again to keep things consistent, was to remove /etc/solid-server. That way the examples follow the same pattern.

There are two other changes I might suggest later: The first, change .db to match the same pattern as the previous two, i.e. (./.db) e.g. /var/www/...

The second, at the very end, start server. There should be at least two options, one using systemctl, the other using solid start.

@Vinnl Vinnl requested a review from ewingson January 14, 2021 08:05
@Vinnl
Copy link
Contributor

Vinnl commented Jan 14, 2021

@ewingson Since you wrote most of this if I recall correctly, would you mind taking a look at this to verify that this looks like you'd expect?

Copy link
Member

@ewingson ewingson left a comment

Choose a reason for hiding this comment

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

thats' s correct, @Vinnl. this is my first review ever. everything changed looks fine, except for it is coded in the init-dialogue of the server (solid init) and there should be changed, too. I' ve documented and tested like in https://gist.github.com/ewingson/c6e97a996aa51eac9f7fd1b7eaf14dc4 and it is a working config.

@Vinnl
Copy link
Contributor

Vinnl commented Jan 14, 2021

Thanks @ewingson. And that's a good point. @xpat, the changed you made are in the output of Node Solid Server, if I understand it correctly, so unless these changes are applied there, the documentation would not match the actual output? Perhaps you could submit a pull request to Node Solid Server as well? On first sight it looks like the required changes would need to be made here: https://github.com/solid/node-solid-server/blob/master/bin/lib/options.js

@xpat
Copy link
Author

xpat commented Jan 14, 2021

@Vinnl @ewingson I submitted a pull request along these lines: Update options.js
#1550

@Vinnl
Copy link
Contributor

Vinnl commented Jan 14, 2021

Thanks! I'll keep an eye on nodeSolidServer/node-solid-server#1550, and if the NSS maintainers merge it, I'll merge the updated instructions here as well.

But are you sure that the changes there really are in line with your changes here? For example, I still see "(with protocol, hostname and port)" in there?

@xpat
Copy link
Author

xpat commented Jan 14, 2021

@Vinnl

But are you sure that the changes there really are in line with your changes here? For example, I still see "(with protocol, hostname and port)" in there?

Yes, I saw that as well. Perhaps I'm not thinking clearly or maybe it's just confusing. The instructions at https://solidproject.org/self-hosting/nss read differently than the actual output, the code for which reads as follows:

{ name: 'server-uri', question: 'Solid server uri (with protocol, hostname and port)', help: "Solid server uri (default: 'https://localhost:8443')", default: 'https://localhost:8443', validate: validUri, prompt: true },

So the actual output "help/default" is at least consistent with the "question" part of the code. Whereas in the instructions, the example is not the same:

Solid server uri (with protocol, hostname and port) https://solidweb.org

In my initial comments, I realized that either we should remove the word "port" or add a port number to the actual/default output. It currently does the latter. So there's no need to change it unless it shouldn't have a port number there. That was one of the most confusing parts for me as a first-time user of the Solid Init command. I assumed that having answered the question SSL port to run on. Default is (8443) 8443, that I didn't have to add a port number to the server uri.

@ewingson
Copy link
Member

ewingson commented Jan 14, 2021

@Vinnl @xpat

let' s work through it.

Solid server uri (with protocol, hostname and port)

at that point we definitely can leave the port out. I used (protocol/hostname) , so https://solidweb.org there and it worked out. so instead of localhost one shall use the URL seen from outside, so the worldviewable URL.

I think the instructions are in fact confusing.

[edit] maybe we should separate or distinguish whether it' s a local install or on a public online root machine.

@xpat
Copy link
Author

xpat commented Jan 14, 2021

@ewingson

maybe what we should separate or distinguish is whether local install or on a public online root machine.

Sounds as if we should. Mind you, I still haven't gotten the Node Solid Server to work anywhere, anyway. I'm just starting out with this as of yesterday and am only now realizing that there's the whole other set of instructions on Github.

@ewingson
Copy link
Member

ewingson commented Jan 14, 2021

@Vinnl @xpat

I guess in case 1 we need
https://localhost:8443 (with port) and in case two
https://domain.com (without port).

but treat that as guess, on my root machine case 2 worked out fine.

@jeff-zucker
Copy link
Member

jeff-zucker commented Jan 14, 2021

https://localhost:8443 (with port)
https://domain.com (without port).

It bears mentioning that the second example is not "without port" it is "without explicitly named port"

@bourgeoa
Copy link
Member

@jeff-zucker I suppose you meant https://domain.com:443

@jeff-zucker
Copy link
Member

LOL, yes, of course.

Base automatically changed from master to main February 7, 2021 16:00
@KyraAssaad
Copy link
Contributor

@ewingson @xpat @Vinnl @jeff-zucker Can someone take a look at this and resolve the conflicts, or close it if we don't want to merge it anymore? Thanks!

@xpat
Copy link
Author

xpat commented Jan 21, 2022

@KyraAssaad I'm not authorized to merge this pull request. I should probably just close it, right? p.s. I eventually did get my own server running (defunct but hopefully soon to be revived at *.solid.mypod.dev).

@xpat xpat closed this Jan 21, 2022
@KyraAssaad
Copy link
Contributor

Ok, thanks @xpat!

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.

6 participants