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

Issues 68 69 #70

Merged
merged 4 commits into from
Sep 12, 2019
Merged

Conversation

JoelPasvolsky
Copy link
Collaborator

Copy link
Member

@randomir randomir left a comment

Choose a reason for hiding this comment

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

Great. Just update the command output (note we don't provide "editable fields" anymore because they could not be made to reliably work on Windows).

above. To get started, Leap users can create a minimum configuration by entering
*only an API token* and accepting the command's defaults for the remaining prompts;
on-premises users should also set the URL to the on-premises system. You can
in the future update the file if needed.
Copy link
Member

Choose a reason for hiding this comment

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

I would note they can accept default by pressing the [Enter] key.

@@ -57,19 +57,19 @@ Client tool installation).
$ dwave config create
Configuration file not found; the default location is: C:\\Users\\jane\\AppData\\Local\\dwavesystem\\dwave\\dwave.conf
Confirm configuration file path (editable):
Profile (create new): prod
API endpoint URL (editable): https://my.dwavesys.url/
Profile (create new):
Copy link
Member

Choose a reason for hiding this comment

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

The command output changed since the time it was captured for docs.

Now it looks like this (on the first run):

$ dwave config create
Configuration file not found; the default location is: /home/radomir/.config/dwave/dwave.conf
Configuration file path [/home/radomir/.config/dwave/dwave.conf]: 
Profile (create new) [prod]: 
API endpoint URL [skip]: 
Authentication token [skip]: asdasdasd
Default client class (qpu or sw) [qpu]: 
Default solver [skip]: 
Configuration saved.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

You removed "Proxy URL"? Thanks, @randomir, I haven't run this for months

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

image

above. To get started, create a minimum configuration by accepting the command's
defaults (pressing Enter) for all prompts except the API token (Leap users) or
API token and endpoint (on-premises users). You can in the future update the
file if needed.
Copy link
Member

Choose a reason for hiding this comment

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

... by either running the same command, and selecting the same profile, or by manually editing the config file.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

@randomir, my preference for this item was to provide minimum detail -- this is a Getting Started step for first-time users, and "You can in the future..." a sentence of guidance for a potential future change. I weigh keeping the Getting Started steps short to encourage users to actually read & follow them over a wall of comprehensive text they will see, groan, and close. Especially, when it comes to a potential future change, which they are unlikely to remember from reading it here, and unlikely to be reading here when the time comes to make that change.

Copy link
Member

Choose a reason for hiding this comment

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

Ok. I thought it would be useful to mention the fact they can edit existing configuration by running the dwave config create command again, especially since you're already talking here how to change the config in the future -- by editing the file (but they don't need to manually edit files). Alternatively, it would be nice to put this our CLI docs, once we have them.

Re: wall of text, may I suggest we:

  • only emphasize the need for API token (in "Interacting with SAPI"), and mention endpoint and solver have default values. Cuts the first section in half.
  • remove the dwave sample example. Adds no value over ping. It should go to CLI docs.
  • remove "properties" examples (both CLI and code) from getting started. Instead maybe just list solvers with dwave solvers -l and mention more details available with dwave solvers?

Also, instead of using "My_DWAVE..." I would keep the output accurate. Same for "http://my.dwavesys.url". Oh, and I think it's important to note that get_solvers example works standalone, without any configuration, because token is the only info needed to run a default Leap solver.

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 suggestions. I put this in #71.
For this PR I just want to clarify, re " since you're already talking here how to change the config in the future -- by editing the file", that the current phrasing is "You can in the future update the file if needed", which includes updating by rerunning dwave CLI rather than just manually editing.

Also worth keeping in mind for the last paragraph that these instructions also apply to on-premises users, so places where we have "http://my.dwavesys.url" are meant for them.

Copy link
Member

Choose a reason for hiding this comment

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

Yes, "update the file" technically includes both scenarios, although (in my head):

  • I update the file by editing it
  • I update the configuration by running a configuration tool (which might write to the same file)

Re: on-prem users. Do you think describing a special case (for a minority of users) in the entry-level getting started guide warrants the distraction it's causing for the majority of Leap users?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

On-premises here was a request from @murraythom. I'm thinking that in the next revision I'd want a separate section for on-premises users with also some additional info such as security certificates, etc

Copy link
Member

Choose a reason for hiding this comment

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

I agree, @JoelPasvolsky, separate section, or even page, makes sense.

@JoelPasvolsky JoelPasvolsky merged commit 58bbacf into dwavesystems:master Sep 12, 2019
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.

2 participants