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

feat: replace netrc with json #146

Open
wants to merge 3 commits into
base: next
Choose a base branch
from

Conversation

alvarosabu
Copy link
Contributor

@alvarosabu alvarosabu commented Jan 8, 2025

This PR replaces the previous approach to save login credentials on ~/.netrc file to a more comfortable ~/.storyblok/credentials.json simplifying the code required and avoiding compatibility issues with other python libraries

Warning

This PR doesn't include multi-region sessions. This will be done in a separate PR.

Closes #137

  • Bugfix
  • Feature
  • Code style update (formatting, renaming)
  • Refactoring (no functional changes, no api changes)
  • Other (please describe):

How to test this PR

Login credentials now should be saved at ~/.storyblok/credentials.json with no disruption on the login/logout functionality

@alvarosabu alvarosabu requested a review from edodusi January 8, 2025 15:05
@alvarosabu alvarosabu added the bugfix [PR] Fixes a bug label Jan 8, 2025
@alvarosabu alvarosabu marked this pull request as draft January 8, 2025 15:06
@alvarosabu alvarosabu changed the title feat: replace netrc with json fix: replace netrc with json Jan 8, 2025
@alvarosabu alvarosabu changed the title fix: replace netrc with json feat: replace netrc with json Jan 8, 2025
@alvarosabu alvarosabu added feature [Issue] New feature or request and removed bugfix [PR] Fixes a bug labels Jan 8, 2025
@alvarosabu alvarosabu self-assigned this Jan 9, 2025
@alvarosabu alvarosabu marked this pull request as ready for review January 9, 2025 15:06
Copy link
Contributor

Choose a reason for hiding this comment

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

adding an empty file here?

}
}

export async function isAuthorized() {
Copy link
Contributor

Choose a reason for hiding this comment

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

this and removeAllNetrcEntries are imported and used in src/commands/logout/index.ts


await initializeSession()

console.log(state)
Copy link
Contributor

Choose a reason for hiding this comment

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

do we need this log here?

const machines = await getNetrcCredentials()
const creds = getCredentialsForMachine(machines, machineName)
// If no environment variables, fall back to .storyblok/credentials.json
const machines = await getCredentials()
Copy link
Contributor

Choose a reason for hiding this comment

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

in a clean environment, launching a login will fail because getCredentials() if it doesn't find the credentials.json throws a filesystem error (handleFileSystemError) that should be catch and handled here

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
feature [Issue] New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants