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

PXP-638: [CLI] Implement new config #183

Merged
merged 21 commits into from
Jan 26, 2024
Merged

PXP-638: [CLI] Implement new config #183

merged 21 commits into from
Jan 26, 2024

Conversation

mfauzaan
Copy link
Member

@mfauzaan mfauzaan commented Jan 5, 2024

Summary

Build the workflow for the wukong init command to satisfy the specifications. Details about the command inputs and outputs can be found here.

Ticket: https://mindvalley.atlassian.net/browse/PXP-524

What's Changed

  • Update the Google auth to store in our config file instead of a separate file
  • Update all auth logics to refer to new path
  • Add google cloud and vault login to wukong init
  • Updated wukong login logic to use same function on wukong init to avoid duplicate codes

@mfauzaan mfauzaan changed the title Feat/implement new config PXP-638: [CLI] Implement new config Jan 5, 2024
@mfauzaan mfauzaan self-assigned this Jan 5, 2024
@mfauzaan mfauzaan added the feature New feature label Jan 5, 2024
@mfauzaan mfauzaan marked this pull request as ready for review January 8, 2024 02:36
@onimsha onimsha requested review from jk-gan and Fadhil January 8, 2024 02:39
Fadhil
Fadhil previously approved these changes Jan 12, 2024
Copy link
Contributor

@Fadhil Fadhil left a comment

Choose a reason for hiding this comment

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

Ok. This looks good to go. We're mostly refactoring some config vars to use the new format with an auth section. Let's test it out.

@jk-gan
Copy link
Contributor

jk-gan commented Jan 15, 2024

As I know, wukong init command should re-initialize the config file. How about the UX in this scenario:
The user had already logged in google cloud and bunker, but they choose to not log in when they run wukong init. Should we remove the existing google cloud and bunker credentials from the newly generated config file?

@onimsha @mfauzaan

@onimsha
Copy link
Contributor

onimsha commented Jan 15, 2024

As I know, wukong init command should re-initialize the config file. How about the UX in this scenario: The user had already logged in google cloud and bunker, but they choose to not log in when they run wukong init. Should we remove the existing google cloud and bunker credentials from the newly generated config file?

@onimsha @mfauzaan

Yeah I think it's the right approach. Because when a user re-init the config, at the end of the initialisation process, they will have to review the config before writing it to disk, so it wouldn't make sense if we keep some of the old configuration.

@sauron-droid
Copy link

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by: mfauzaan, onimsha

The full list of commands accepted by this bot can be found here.

Needs approval from an approver in each of these files:

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@onimsha onimsha merged commit 71ae236 into main Jan 26, 2024
5 checks passed
@sauron-droid sauron-droid deleted the feat/implement-new-config branch January 26, 2024 02:39
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Development

Successfully merging this pull request may close these issues.

5 participants