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

Add helper output for connecting after running add-to-ssh-agent #32

Closed
wants to merge 10 commits into from
Closed

Add helper output for connecting after running add-to-ssh-agent #32

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
10 commits
Select commit Hold shift + click to select a range
4fedbb1
Add helper output for connecting after running add-to-ssh-agent
imrehg Mar 19, 2020
b651327
Formatting changes, not connected to the main code, required by black
imrehg Mar 19, 2020
d5da1d8
Redo string output with format
imrehg Mar 19, 2020
fe1bfcb
Update flake8 check to follow black's formatting
imrehg Mar 19, 2020
53d53d2
Revert "Update flake8 check to follow black's formatting" due to Pyth…
imrehg Mar 19, 2020
a0209d7
Simplify output formatting and linter setup.
imrehg Mar 19, 2020
2e8eec9
More formatting, as newer black does things differently.
imrehg Mar 19, 2020
90b46ff
Remove ineffective flake8 setting, update grammar, reformat string
imrehg Mar 21, 2020
1d6d9a9
Instruct flake8 to ignore long line errors, and string formatting
imrehg Mar 21, 2020
ef3e309
Add another ignore to flake8, for usability and relying on black
imrehg Mar 21, 2020
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
15 changes: 11 additions & 4 deletions faculty_cli/cli.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -735,6 +735,13 @@ def ssh(project, server):
tablefmt="plain",
)
)
click.echo(
"\n"
+ "Login to the server with:\n"
+ "ssh {}@{} -p {} -o UserKnownHostsFile={} -o StrictHostKeyChecking=no".format(
details.username, details.hostname, details.port, os.devnull
)
)
Comment on lines +738 to +744
Copy link
Member

Choose a reason for hiding this comment

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

Might this be a little tidier as:

Suggested change
click.echo(
"\n"
+ "Login to the server with:\n"
+ "ssh {}@{} -p {} -o UserKnownHostsFile={} -o StrictHostKeyChecking=no".format(
details.username, details.hostname, details.port, os.devnull
)
)
click.echo("")
click.echo("Login to the server with:")
click.echo(
"ssh {}@{} -p {} -o UserKnownHostsFile={} -o StrictHostKeyChecking=no".format(
details.username, details.hostname, details.port, os.devnull
)
)

We already compose the output like this in https://github.com/facultyai/faculty-cli/blob/master/faculty_cli/cli.py#L851

Copy link
Member Author

Choose a reason for hiding this comment

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

Ah, thanks for the example! I was wondering of a setup like this, but haven't looked deep enough. Makes complete sense!



@cli.command(context_settings={"ignore_unknown_options": True})
Expand Down Expand Up @@ -1158,7 +1165,7 @@ def put(project, local, remote, server):
"-P",
str(details.port),
os.path.expanduser(local),
u"{}@{}:{}".format(
"{}@{}:{}".format(
Copy link
Member

Choose a reason for hiding this comment

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

We've not yet made the decision to drop Python 2 support. Can we leave these unicode literals until then?

Copy link
Member Author

Choose a reason for hiding this comment

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

Looking back through the changes, I believe this is done by a newer version of black (than defined in tox.ini) cleaning that up, and when I realised that I run locally a different version and downgraded, I didn't go back to undo the changes that the original (version 19?) done. Totally makes sense, and will watch out for this later.

details.username, details.hostname, escaped_remote
),
]
Expand Down Expand Up @@ -1190,7 +1197,7 @@ def get(project, remote, local, server):
filename,
"-P",
str(details.port),
u"{}@{}:{}".format(
"{}@{}:{}".format(
details.username, details.hostname, escaped_remote
),
os.path.expanduser(local),
Expand All @@ -1210,11 +1217,11 @@ def _rsync(project, local, remote, server, rsync_opts, up):
escaped_remote = faculty_cli.shell.quote(remote)
if up:
path_from = local
path_to = u"{}@{}:{}".format(
path_to = "{}@{}:{}".format(
details.username, details.hostname, escaped_remote
)
else:
path_from = u"{}@{}:{}".format(
path_from = "{}@{}:{}".format(
details.username, details.hostname, escaped_remote
)
path_to = local
Expand Down
5 changes: 5 additions & 0 deletions tox.ini
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -22,3 +22,8 @@ deps =
black==18.9b0
commands =
black {posargs:--check setup.py faculty_cli test}

[flake8]
# Ignore long line errors, and rely on the formatting done properly by black
# Also ignore "Line break occurred before a binary operator" also coming from black
ignore = E501,W503
Copy link
Member

Choose a reason for hiding this comment

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

Can we instead put this in setup.cfg, as in https://github.com/facultyai/faculty/blob/master/setup.cfg ?

It should then get picked up by linter plugins in peoples' editors.

Copy link
Member Author

Choose a reason for hiding this comment

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

That's good info, thanks! Just getting to learn the number of different configs that one can use to set these tooling (just like pyproject.toml mentioned earlier). Will try that out too, and it makes complete sense putting it where it has the best effect.