-
Notifications
You must be signed in to change notification settings - Fork 11
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
feat: apkam-aware activate_cli #576
Conversation
…tPreference fields
# Conflicts: # packages/at_onboarding_cli/bin/activate_cli.dart # packages/at_onboarding_cli/pubspec.yaml
…; deprecate the int? pkamRetryIntervalMins parameter
…nagement. Interim commit: mostly structural changes for testability, readability etc
…lint rules to analysis_options.yaml including the unawaited futures rule so we don't have this happen anywhere else.
… go to prod swarms before can be used for prod
…y / revoke in auth_cli
…om pub.dev rather than local path. This is slightly inconvenient but we have a circular dependency now between at_onboarding_cli and at_cli_commons.
final Duration retryInterval = Duration(minutes: pkamRetryIntervalMins); | ||
retryInterval ??= Duration( | ||
minutes: pkamRetryIntervalMins ?? | ||
atOnboardingPreference.apkamAuthRetryDurationMins); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
added retryInterval as a Duration, so calling code has more control
@@ -160,11 +166,11 @@ class AtOnboardingServiceImpl implements AtOnboardingService { | |||
atLookUpImpl.atChops = AtChopsImpl(atChopsKeys); | |||
|
|||
// Pkam auth will be attempted asynchronously until enrollment is approved/denied | |||
_attemptPkamAuthAsync( | |||
await _attemptPkamAuthAsync( |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
added missing await
@@ -174,12 +180,13 @@ class AtOnboardingServiceImpl implements AtOnboardingService { | |||
return enrollmentResponse; | |||
} | |||
|
|||
void _listenToPkamSuccessStream( | |||
Future<void> _listenToPkamSuccessStream ( |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Made _listenToPkamSuccessStream return a Future which is only completed when we've actually processed the success
|
||
import 'package:args/args.dart'; | ||
|
||
extension PrintAllArgParserUsage on ArgParser { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
A little extension to help with pretty printing of the auth_cli's ArgParsers' usage when there are subcommands involved
final first = arguments.first; | ||
if (first.startsWith('-') && first != '-h' && first != '--help') { | ||
// no command found ... legacy ... insert 'onboard' as the command | ||
arguments = ['onboard', ...arguments]; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is the only tricky bit of logic to be aware of, to allow us to most easily provide backwards compatibility
Can we log the enrollment response from server after running the above command. I had to copy the enrollmentId from the server logs and pass it to the approve command. |
With dart 3.3.4, I am getting this warning every time I run activate_cli
|
Should not occur if you do |
Done in latest commit |
…o some refactoring - Deprecated `AtOnboardingPreference.apkamAuthRetryDurationMins` - Added three new methods to AtOnboardingService - sendEnrollRequest, awaitApproval and createAtKeysFile. Extracted those methods from AtOnboardingServiceImpl.enroll so it now just calls those three methods. The refactoring provides more control to application code. - For readability, refactored the code related to pkam auth in the context of enrollment success checking
@murali-shris please can you re-review? |
- What I did
- How I did it
- How to verify it
dart bin/activate_cli.dart -h
to see overall usage info and list of available commandsdart bin/activate_cli.dart <some command> -h
to see help and usage info for a specific commanddart bin/activate_cli.dart otp -a @alice
to generate an OTP for exampledart bin/activate_cli.dart interactive -a @alice
to run interactively-Additional context