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

Enhance docs of 'sourced init' and 'web' subcommands #115

Merged
merged 2 commits into from
Jul 2, 2019
Merged

Enhance docs of 'sourced init' and 'web' subcommands #115

merged 2 commits into from
Jul 2, 2019

Conversation

dpordomingo
Copy link
Contributor

@dpordomingo dpordomingo added the enhancement New feature or request label Jun 27, 2019
@dpordomingo dpordomingo requested review from EgorBu and a team June 27, 2019 18:47
@dpordomingo dpordomingo self-assigned this Jun 27, 2019
@carlosms
Copy link
Contributor

I think this would also be useful to have in the CLI help output.

@smacker
Copy link
Contributor

smacker commented Jun 28, 2019 •
edited
Loading

Imo it should be even more useful to print after init or during the init (because browser most probably will be open over terminal).

@dpordomingo
Copy link
Contributor Author

The purpose of this PR is only to enhance the docs.
Enhancing the init and web commands should be done separately.

Should it be hinted after the init succeeds, or before trying opening the browser? Is the command aware of the password defined in the docker-compose.yml that is used by the UI?

EgorBu
EgorBu reviewed Jul 2, 2019
View reviewed changes
Copy link

@EgorBu EgorBu left a comment

Choose a reason for hiding this comment

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

Thanks a lot for the fast reply and great work! Just a couple of questions

docs/usage/commands.md Outdated
@@ -22,6 +22,11 @@ It can work over a local repository or a list of GitHub organizations.

**source{d} CE** will download and install Docker images on demand. Therefore, the first time you run some of these commands, they might take a bit of time to start up. Subsequent runs will be faster.

Once **source{d} CE** has been initialized, it will automatically open the web UI.
If the UI is not opened automatically, you can use [`sourced web`](#sourced-web) command, or visit http://localhost:8088.
Copy link

Choose a reason for hiding this comment

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

Should it be 127.0.0.1? #116

Copy link

Choose a reason for hiding this comment

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

Btw - it would be nice to have --no-browser option. I forgot to mention in friction log that this command launched a browser without tab localhost:8088 - so I had to copy-paste it anyway.

Copy link
Contributor Author

@dpordomingo dpordomingo Jul 2, 2019 •
edited
Loading

Choose a reason for hiding this comment

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

Thanks for noticing that it should use 127.0.0.1 instead of localhost
About the second comment, it should be handled on a separate issue, but:

  • what would be the use case of --no-browser? when would you want to avoid opening the UI?
  • is the browser always opened without url? which browsers do you have? which OS? what happens if you run xdg-open http://127.0.0.1:8088 in your Linux (or open http://127.0.0.1:8088 in Darwin)?

Copy link

Choose a reason for hiding this comment

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

what would be the use case of --no-browser? when would you want to avoid opening the UI?

Like jupyter - some kind of server application - you launch it, expose port, so external people can connect.

is the browser always opened without url?

  • Yes, somehow something is misconfigured on my PC

which browsers do you have?

  • Chromium Version 74.0.3729.169 (Official Build) (64-bit)

which OS?

Distributor ID:	Ubuntu
Description:	Ubuntu 16.04.6 LTS
Release:	16.04
Codename:	xenial

what happens if you run xdg-open http://127.0.0.1:8088 in your Linux (or open http://127.0.0.1:8088 in Darwin)?

  • browser is opened with empty tab

Copy link
Contributor Author

Choose a reason for hiding this comment

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

So that is the problem. sourced web relies on xdg-open, which is not working in your PC, maybe because of a misconfigured Chrome as explained by https://askubuntu.com/questions/499200/xdg-open-opens-chrome-but-does-not-load-the-url
Could you confirm?

Copy link

Choose a reason for hiding this comment

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

Yes, it was misconfigured. Thank you for fix!

EgorBu
EgorBu approved these changes Jul 2, 2019
View reviewed changes
Copy link

@EgorBu EgorBu left a comment

Choose a reason for hiding this comment

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

👍

@carlosms
Copy link
Contributor

carlosms commented Jul 2, 2019

The purpose of this PR is only to enhance the docs.
Enhancing the init and web commands should be done separately.

Why? this is such a small PR, and the requested changes are very very related, it makes sense to keep in sync the docs and the CLI help messages

Should it be hinted after the init succeeds, or before trying opening the browser? Is the command aware of the password defined in the docker-compose.yml that is used by the UI?

I'd say it's ok to output the "default" password and not try to parse the compose file.
As for before of after, both should be fine in my opinion. Maybe before will have more chances to be seen, since the browser will maybe hide the terminal when it's opened.

@dpordomingo
Copy link
Contributor Author

dpordomingo commented Jul 2, 2019 •
edited
Loading

.

@dpordomingo dpordomingo closed this Jul 2, 2019
@dpordomingo dpordomingo reopened this Jul 2, 2019
@dpordomingo
Copy link
Contributor Author

dpordomingo commented Jul 2, 2019 •
edited
Loading

@carlosms @smacker your suggestion was added by 627e345

$ sourced web --help
Usage:
  sourced [OPTIONS] web [web-OPTIONS]

Open the web interface in your browser, by default at: http://127.0.0.1:8088 user:admin pass:admin

$ sourced init local
docker-compose working directory set to /projects/github.com/src-d/sourced-ce
Starting srcd-093c09ad4231381f4891a517a6ab5a7a41021519_metadatadb_1 ... done
Starting srcd-093c09ad4231381f4891a517a6ab5a7a41021519_redis_1      ... done
Starting srcd-093c09ad4231381f4891a517a6ab5a7a41021519_bblfsh_1     ... done
Starting srcd-093c09ad4231381f4891a517a6ab5a7a41021519_postgres_1   ... done
Starting srcd-093c09ad4231381f4891a517a6ab5a7a41021519_ghsync_1       ... done
Starting srcd-093c09ad4231381f4891a517a6ab5a7a41021519_bblfsh-web_1   ... done
Starting srcd-093c09ad4231381f4891a517a6ab5a7a41021519_gitbase_1      ... done
Starting srcd-093c09ad4231381f4891a517a6ab5a7a41021519_gitcollector_1 ... done
Starting srcd-093c09ad4231381f4891a517a6ab5a7a41021519_sourced-ui_1   ... done

Once source{d} is fully initialized, the UI will be available, by default at:
  http://127.0.0.1:8088
  user:admin
  pass:admin
	
Initializing source{d}... ⠸

$ sourced start
Starting bblfsh       ... done
Starting gitbase      ... done
Starting bblfsh-web   ... done
Starting redis        ... done
Starting postgres     ... done
Starting gitcollector ... done
Starting metadatadb   ... done
Starting ghsync       ... done
Starting sourced-ui   ... done

Once source{d} is fully initialized, the UI will be available, by default at:
  http://127.0.0.1:8088
  user:admin
  pass:admin
	
Initializing source{d}... ⠹

$ sourced web

Once source{d} is fully initialized, the UI will be available, by default at:
  http://127.0.0.1:8088
  user:admin
  pass:admin

@dpordomingo dpordomingo merged commit 89a607e into src-d:master Jul 2, 2019
@dpordomingo dpordomingo deleted the enhance-docs-init branch July 2, 2019 15:21
@dpordomingo dpordomingo mentioned this pull request Jul 3, 2019
Open
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@EgorBu EgorBu EgorBu approved these changes

@carlosms carlosms carlosms approved these changes

@smacker smacker smacker approved these changes

Assignees

@dpordomingo dpordomingo

Labels
enhancement New feature or request
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@dpordomingo @carlosms @smacker @EgorBu