You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@spark.apache.org by j-martin <gi...@git.apache.org> on 2014/03/30 03:15:02 UTC
[GitHub] spark pull request: Listing of instances to be terminated before t...
GitHub user j-martin opened a pull request:
https://github.com/apache/spark/pull/270
Listing of instances to be terminated before the prompt
Will list the EC2 instances before detroying the cluster.
This was added because it can be scary to destroy EC2
instances without knowing which one will be impacted.
You can merge this pull request into a Git repository by running:
$ git pull https://github.com/j-martin/spark master
Alternatively you can review and apply these changes as the patch at:
https://github.com/apache/spark/pull/270.patch
To close this pull request, make a commit to your master/trunk branch
with (at least) the following in the commit message:
This closes #270
----
commit 27b0a366062e6cbc4700ea2f37e31059561dc80b
Author: Jean-Martin Archer <je...@pulseenergy.com>
Date: 2014-03-30T01:13:32Z
Listing of instances to be terminated before the prompt
Will list the EC2 instances before detroying the cluster.
This was added because it can be scary to destroy EC2
instances without knowing which one will be impacted.
----
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] spark pull request: SPARK-2166 - Listing of instances to be termin...
Posted by asfgit <gi...@git.apache.org>.
Github user asfgit closed the pull request at:
https://github.com/apache/spark/pull/270
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] spark pull request: Listing of instances to be terminated before t...
Posted by pwendell <gi...@git.apache.org>.
Github user pwendell commented on the pull request:
https://github.com/apache/spark/pull/270#issuecomment-46341757
Made some minor comments. Overall, this seems like a great to have.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] spark pull request: Listing of instances to be terminated before t...
Posted by pwendell <gi...@git.apache.org>.
Github user pwendell commented on the pull request:
https://github.com/apache/spark/pull/270#issuecomment-46341975
@j-martin one other thing. Would you mind creating a brief JIRA to describe this feature? You can just create an account on the site. Many people follow our issue tracker so we try to have JIRA's for new things:
https://issues.apache.org/jira/browse/SPARK
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] spark pull request: Listing of instances to be terminated before t...
Posted by pwendell <gi...@git.apache.org>.
Github user pwendell commented on a diff in the pull request:
https://github.com/apache/spark/pull/270#discussion_r13874441
--- Diff: ec2/spark_ec2.py ---
@@ -691,12 +691,18 @@ def real_main():
setup_cluster(conn, master_nodes, slave_nodes, opts, True)
elif action == "destroy":
- response = raw_input("Are you sure you want to destroy the cluster " +
- cluster_name + "?\nALL DATA ON ALL NODES WILL BE LOST!!\n" +
+ (master_nodes, slave_nodes) = get_existing_cluster(
+ conn, opts, cluster_name, die_on_error=False)
+
+ print "Are you sure you want to destroy the cluster " + \
+ cluster_name + "?\nThe following instances will be terminated:"
+ for inst in master_nodes + slave_nodes:
+ print inst
--- End diff --
in other locations where we print out nodes, we usually print `inst.public_dns_name` would it make sense to do the same here?
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] spark pull request: SPARK-2166 - Listing of instances to be termin...
Posted by pwendell <gi...@git.apache.org>.
Github user pwendell commented on the pull request:
https://github.com/apache/spark/pull/270#issuecomment-46765110
@j-martin Seems like this doesn't merge cleanly. Do you mind up-merging it?
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] spark pull request: Listing of instances to be terminated before t...
Posted by pwendell <gi...@git.apache.org>.
Github user pwendell commented on a diff in the pull request:
https://github.com/apache/spark/pull/270#discussion_r13874310
--- Diff: ec2/spark_ec2.py ---
@@ -691,12 +691,18 @@ def real_main():
setup_cluster(conn, master_nodes, slave_nodes, opts, True)
elif action == "destroy":
- response = raw_input("Are you sure you want to destroy the cluster " +
- cluster_name + "?\nALL DATA ON ALL NODES WILL BE LOST!!\n" +
+ (master_nodes, slave_nodes) = get_existing_cluster(
+ conn, opts, cluster_name, die_on_error=False)
+
+ print "Are you sure you want to destroy the cluster " + \
--- End diff --
Very minor - but it might be slightly nicer to write this using string interpolation and also just using two different print statements instead of putting a newline directly in there (since you'll get python's system-dependent line delimiter).
```
print "Are you sure you want to destroy the cluster %s?" % cluster_name
print "The following instances will be terminated:"
```
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] spark pull request: SPARK-2166 - Listing of instances to be termin...
Posted by pwendell <gi...@git.apache.org>.
Github user pwendell commented on the pull request:
https://github.com/apache/spark/pull/270#issuecomment-46804399
This was actually a pretty tough merge since we changed the spacing around a lot in `spark_ec2` recently. I went ahead and manually dealt with the merge. I also made two minor changes on merge.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] spark pull request: Listing of instances to be terminated before t...
Posted by j-martin <gi...@git.apache.org>.
Github user j-martin commented on the pull request:
https://github.com/apache/spark/pull/270#issuecomment-46349745
I created the issue. https://issues.apache.org/jira/browse/SPARK-2166
I will also apply the recommendations to the PR shortly.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---