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

Add password getter system for customize getting password. (#298) #299

Merged
merged 8 commits into from
Jun 4, 2015
Merged

Conversation

ZhukV
Copy link
Contributor

@ZhukV ZhukV commented May 26, 2015

  1. Add AskPasswordGetter (ask password before connecting)
  2. Add CallabackPasswordGetter (gets password with call callbacks, Closures)

For more information, please see #298

1. Add AskPasswordGetter (ask password before connecting)
2. Add CallabackPasswordGetter (gets password with call callbacks, Closures)
@antonmedv
Copy link
Member

Cool! 👍
Need more time to test, try to do all on weekend.
Please, take a look at scrutinizer report for your PR: https://scrutinizer-ci.com/g/deployphp/deployer/inspections/a5866db1-224d-46b3-ad6a-e2e3e6f00568

2015-05-27 12 33 53

@ZhukV
Copy link
Contributor Author

ZhukV commented May 27, 2015

Ok. I not see composer.json.

"symfony/console": "~2.6"

And i checks the QuestionHelper class exists for indicate SymfonyConsole of > 2.5 version.

@antonmedv
Copy link
Member

Test coverage decreaseв a little bit. If you can keep them on same level, it will be 100% awesome 👍 But if not, nothing terrible, leave it as it is.

PS Where are a lot issue to implement too, if you want take a look at #297.
PPS Soon Deployer Roadmap will be ready.

@Max-Might
Copy link
Contributor

I think PhpSecLib used to ask for password if it was empty, is that correct?

@ZhukV
Copy link
Contributor Author

ZhukV commented May 27, 2015

@Max-Might Yes, because password prompt before connection (in configuraiton Configuration::getPassword). The this feature will be worked in any 'adapters' for connection.

Example for ask password:

server('production', 'domain.com')
    ->user('user')
    ->password() // <-- Default null (If null, the AskPasswordGetter will be created automatically)
    ->stage('production')
    ->env('deploy_path', '/var/www/domain.com');

@antonmedv
Copy link
Member

Cool feature.

@DirtyB
Copy link

DirtyB commented May 27, 2015

Will this work for passphrase in identityFile function?

@ZhukV
Copy link
Contributor Author

ZhukV commented May 27, 2015

@DirtyB, no, but this easy add.

@antonmedv
Copy link
Member

antonmedv commented May 27, 2015 via email

@ZhukV
Copy link
Contributor Author

ZhukV commented May 27, 2015

ok.

@ZhukV
Copy link
Contributor Author

ZhukV commented May 27, 2015

Adds.

Attention: by default, passphrase - '' (empty string). For use password getter automatically, you must set null for pass phrase.

->identityFile(null, null, null)

P.S. Can set the null value for $passPhrase as default?

@antonmedv
Copy link
Member

Thats ok, pass must be ‘’ as default.

@antonmedv antonmedv closed this May 28, 2015
@antonmedv antonmedv reopened this May 28, 2015
@antonmedv antonmedv closed this May 28, 2015
@antonmedv antonmedv reopened this May 28, 2015
@antonmedv
Copy link
Member

Nice work. Need same one else too look at the PR before i'll merge it.

@ZhukV
Copy link
Contributor Author

ZhukV commented Jun 3, 2015

ping

@antonmedv
Copy link
Member

I tested this feature, look good. Guys, please, test this feature too!

antonmedv pushed a commit that referenced this pull request Jun 4, 2015
Add password getter system for customize getting password. (#298)
@antonmedv antonmedv merged commit b9b118d into deployphp:master Jun 4, 2015
@antonmedv
Copy link
Member

@ZhukV i'm going to implement event dispatcher in Deployer. Can you rewrite your feature by using dispatcher later?

@ZhukV
Copy link
Contributor Author

ZhukV commented Jun 4, 2015

👍 No problem!

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

Successfully merging this pull request may close these issues.

None yet

4 participants