Skip to content
This repository has been archived by the owner on Aug 22, 2023. It is now read-only.

#16 Interactive mode #18

Merged
merged 9 commits into from
Jun 27, 2016
Merged

#16 Interactive mode #18

merged 9 commits into from
Jun 27, 2016

Conversation

outime
Copy link
Contributor

@outime outime commented Jun 16, 2016

No description provided.

@outime outime mentioned this pull request Jun 16, 2016
@coveralls
Copy link

Coverage Status

Coverage decreased (-13.4%) to 65.799% when pulling 74084fb on outime:issue-16 into 1b3a817 on zalando-stups:master.


content = ''.join(tracebacks)

with NamedTemporaryFile(prefix="senza-traceback-", delete=False) as error_file:
Copy link
Contributor

Choose a reason for hiding this comment

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

senza?

@hjacobs
Copy link
Contributor

hjacobs commented Jun 16, 2016

Please add tests.

'''Request SSH access to a single host'''

if interactive:
ec2 = boto3.resource('ec2')
reservations = ec2.instances.filter(
Copy link
Contributor

Choose a reason for hiding this comment

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

you need to add paging support here

Copy link
Contributor

Choose a reason for hiding this comment

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

or maybe not (?) --- I usually use the low-level "clients"

@outime
Copy link
Contributor Author

outime commented Jun 16, 2016

@hjacobs thanks for all the suggestions, I find them reasonable and will be done next. I had tests in mind but wanted to make sure about the implementation first.

@coveralls
Copy link

Coverage Status

Coverage decreased (-14.9%) to 64.26% when pulling 0bc80a2 on outime:issue-16 into 1b3a817 on zalando-stups:master.

@outime
Copy link
Contributor Author

outime commented Jun 17, 2016

@hjacobs tests pass in my local computer (tried with Python 3.4.x and 3.5.x) but seems stuck in Travis CI. Would you mind taking a look to that as well as the rest of the code? On the other hand, seems that resources handle pagination for you:

A collection seamlessly handles pagination for you, making it possible to easily iterate over all items from all pages of data. (source)

@@ -280,8 +280,8 @@ def request_access(obj, host, reason, reason_cont, user, password, even_url, odd


def request_access_interactive():
region_name = click.prompt('AWS region', default=os.getenv('PIU_REGION') or
subprocess.getoutput('aws configure get region'))
region_name = os.getenv('PIU_REGION') or click.prompt('AWS region',
Copy link
Contributor

Choose a reason for hiding this comment

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

ohoh, please check how Senza does it: AWS_DEFAULT_REGION and ~/.aws/config is honored

@outime
Copy link
Contributor Author

outime commented Jun 20, 2016

@hjacobs done, is this better?

@hjacobs
Copy link
Contributor

hjacobs commented Jun 24, 2016

@tuxlife any opinion on this?

@tuxlife
Copy link
Contributor

tuxlife commented Jun 26, 2016

@outime Why did you need a interactive mode?

@outime
Copy link
Contributor Author

outime commented Jun 26, 2016

@tuxlife it's much more comfortable to use Più this way since you don't have to look up for the private IP of the instance with Senza, copy it, then execute Più, etc. Here you get a list of instances for the specified region and choose the one you want easily without caring about IP addresses. I use it with the --connect flag which makes it even easier to use as it connects right away.

On the other hand, I've met some Zalando folks who are using a custom wrapper with similar functionality. I also use one. It feels natural to have this feature built-in to have similar workflow.

@szuecs
Copy link
Member

szuecs commented Jun 27, 2016

👍

1 similar comment
@tuxlife
Copy link
Contributor

tuxlife commented Jun 27, 2016

👍

@tuxlife tuxlife merged commit 4680ad5 into zalando-stups:master Jun 27, 2016
@outime
Copy link
Contributor Author

outime commented Jun 28, 2016

Will submit PR for docs when the newer package is published :)

@outime
Copy link
Contributor Author

outime commented Jul 6, 2016

Could any of the maintainers update the PyPI package whenever you have some time? I've also written some docs: zalando-stups/documentation#89

@hjacobs
Copy link
Contributor

hjacobs commented Jul 6, 2016

@outime this just does not work on Travis nor our CD:

tests/test_cli.py::test_interactive_success 
No output has been received in the last 10m0s, this potentially indicates a stalled build or something wrong with the build itself.

@outime
Copy link
Contributor Author

outime commented Aug 1, 2016

@hjacobs I was on holidays so I couldn't check this. Now I see it's fixed (thanks @mikkeloscar) so hopefully the package can now be updated.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants