spark-reviews mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From kanzhang <>
Subject [GitHub] spark pull request: [SPARK-8129] [CORE] [SECURITY] Securely pass a...
Date Fri, 12 Jun 2015 01:36:52 GMT
Github user kanzhang commented on the pull request:
    Thanks. So it's not just 3 lines. :-) Honestly, if you finish it up and add similar tests
as I did, you will end up with less but not substantially less code. Apart from a few straight-forward
convenience methods in SparkConf and Utils, the rest of my non-test changes are minimal per
file or cosmetic. Most of the patch is either tests or refactoring of code chunks into their
own methods to support testing (e.g., in `Client`, `SparkDeploySchedulerBackend`).
    I found a simpler way to fix it if I don't introduce the concept of per-app key, which
is to do all the filtering and setting env variables in `buildLocalCommand`. Thank you for
bringing it to my attention earlier. It allows me handle all 3 cases in one place. I just
need one set of tests instead of 3, and no code refactoring is needed for testing. I'll submit
a new PR. You are welcome to review it.
    Let's leave this PR for collecting feedbacks on my stdin approach. I admit I'm guilty
of trying to squeeze in more code than necessary to fix the issue in the current model (the
`appSecret` stuff). But I was gearing toward per-app secrets with my stdin approach.

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 or file a JIRA ticket
with INFRA.

To unsubscribe, e-mail:
For additional commands, e-mail:

View raw message