Adding workspace/account logic for python #81

IaroslavTitov · 2025-03-19T18:26:53Z

Summary

Fixes Add logic in python SDK to use ESC accounts #79
Adding credentials loading system into Python SDK
This is not 1:1 with ESC CLI capabilities, only the necessary parts for SDK to load credentials
The default configuration will now look for env vars first, then fallback onto accounts (the same behavior as CLI)

Testing

Adding new tests, verifying file loading and parsing logic
Manually tested using existing creds file setup, running tests on Pulumi prod and review stack, switching between them only using CLI

Note: I'm not a Python expert, so please correct any conventions I missed

cleverguy25 · 2025-03-19T20:01:16Z

sdk/python/pulumi_esc_sdk/esc_client.py

@@ -441,6 +442,14 @@ def default_config(host=None,
        access_token = os.getenv("PULUMI_ACCESS_TOKEN")
    if not host:
        host = os.getenv("PULUMI_BACKEND_URL")
+
+    account, backend_url = workspace.get_current_account(False)


Should we check whether access token or host because this is probing the file system and we should avoid it if we have what we need?

Good point, will check if either accessToken or backend URL is missing before loading account

seanyeh · 2025-03-19T20:12:22Z

sdk/python/pulumi_esc_sdk/workspace.py

lets mark this and the workspace_models.py as linguist-generated=false in .gitattributes

komalali · 2025-03-19T21:49:58Z

CHANGELOG_PENDING.md

@@ -4,6 +4,8 @@
  - All SDK now automatically read in configuration environment variables
  - Go SDK also automatically picks up configuration from CLI Pulumi accounts
  [#76](https://github.com/pulumi/esc-sdk/pull/76)
+- Adding CLI Pulumi accounts loading logic into Python SDK


Suggested change

- Adding CLI Pulumi accounts loading logic into Python SDK

- Adds support for reading credentials from disk to Python SDK

komalali · 2025-03-19T21:53:35Z

sdk/python/pulumi_esc_sdk/workspace.py

@@ -0,0 +1,89 @@
+# Copyright 2025, Pulumi Corporation.  All rights reserved.
+
+"""Recreating Pulumi workspace and account logic for python SDK."""


I'd also recommend adding a reference to the go implementation here so we know what this is implementing and make it clear that we are porting over some existing code.

Suggested change

"""Recreating Pulumi workspace and account logic for python SDK."""

"""Pulumi workspace and account logic for python SDK."""

Good idea, will link to ESC CLI code

komalali · 2025-03-19T21:58:42Z

sdk/python/pulumi_esc_sdk/workspace.py

+    # Otherwise, use the current user's home dir + .pulumi
+    home_dir = pathlib.Path.home()
+
+    return str(home_dir / ".pulumi")


Suggested change

return str(home_dir / ".pulumi")

return str(home_dir.joinpath(".pulumi"))

komalali · 2025-03-19T22:04:00Z

sdk/python/pulumi_esc_sdk/workspace.py

+def get_esc_bookkeeping_dir() -> str:
+    """
+    Returns the path of the '.esc' folder inside Pulumi home dir.
+    """
+    return get_pulumi_path(".esc")


What is that directory for? I don't see it on my machine.

This one appears if you login via esc cli, not the pulumi one. It is a single-value file like this

{ "name": "https://api.pulumi.com/" }

that allows you to override current account. So if your current account in normal credentials file is moolumi.com, and you use pulumi CLI, that's what it will use. But if using esc CLI you then login to boolumi.com, it will use that as default in ESC CLI. See this method as go reference.

Okay sweet can we just add a comment to that effect?

komalali · 2025-03-19T22:04:29Z

sdk/python/pulumi_esc_sdk/workspace.py

+    """
+    Returns the path to the esc credentials file on disk.
+    """
+    return str(pathlib.Path(dir) / "credentials.json")


Suggested change

return str(pathlib.Path(dir) / "credentials.json")

return str(pathlib.Path(dir).joinpath("credentials.json"))

komalali · 2025-03-19T22:07:41Z

sdk/python/pulumi_esc_sdk/workspace_models.py

@@ -0,0 +1,41 @@
+# Copyright 2025, Pulumi Corporation.  All rights reserved.
+
+"""Pulumi workspace models."""


Reference the file/code this is porting

komalali · 2025-03-19T22:08:12Z

sdk/python/test/test_pulumi_home/credentials.json

+            "lastValidatedAt": "2026-03-17T11:19:28.392525488-06:00"
+        }
+    }
+}


Suggested change

}

}

komalali · 2025-03-19T22:08:21Z

sdk/python/test/test_pulumi_home_esc/.esc/credentials.json

@@ -0,0 +1,3 @@
+{
+    "name": "https://api.boolumi.com"
+}


Suggested change

}

}

komalali · 2025-03-19T22:08:31Z

sdk/python/test/test_pulumi_home_esc/credentials.json

+            "lastValidatedAt": "2026-03-17T11:19:28.392525488-06:00"
+        }
+    }
+}


Suggested change

}

}

komalali · 2025-03-19T22:08:42Z

sdk/python/test/test_workspace_accounts.py

+        self.assertTrue(self.config.api_key['Authorization'], "pul-fake-token-boo")
+
+if __name__ == '__main__':
+    unittest.main()


Suggested change

unittest.main()

unittest.main()

Thank you for the thorough review, fixed up these issues!

seanyeh

mostly small things, looks good otherwise

seanyeh · 2025-03-20T13:23:29Z

sdk/python/pulumi_esc_sdk/workspace.py

+
+    return str(home_dir.joinpath(".pulumi"))
+
+def get_pulumi_path(*elem: str) -> str:


I see this method is only called with ".esc" and with no args. can the no args call be replaced with get_pulumi_home_dir()? If so, then I don't think this method has any purpose

Fair point, I was porting over method by method, and in CLI there's more uses for it. But in this case, I don't expect more usage, so can eliminate this for sure.

seanyeh · 2025-03-20T13:24:28Z

sdk/python/pulumi_esc_sdk/workspace.py

+    """
+    creds_file = get_path_to_creds_file(get_esc_bookkeeping_dir())
+    try:
+        with open(creds_file, 'r') as f:


nit: let's be consistent with using double quotes, here and elsewhere

seanyeh · 2025-03-20T13:25:35Z

sdk/python/pulumi_esc_sdk/workspace.py

+    try:
+        with open(creds_file, 'r') as f:
+            data = json.loads(f.read())
+            return data['name']


we should add a case for if name doesn't exist. whether that's a conditional here or caught as an except ..., either is fine to me

Good point, no reason to print "unexpected error" when it is rather expected. I'll make it return None on KeyError

seanyeh · 2025-03-20T13:33:18Z

sdk/python/pulumi_esc_sdk/workspace_models.py

+    accounts: Dict[str, Account]
+
+    @classmethod
+    def from_json(self, dataJson: str):


Account.from_json reads from a dict, but this reads from a json string? maybe we can name this from_file and pass the file? (json.loads can accept a file so we can skip the extra f.read() step)

Good point, not great that they are different. I think it would be better to keep the methods the same, both from_jsons receiving dict with values, and we can follow the same pattern of data = json.loads(f.read()) on file loading

seanyeh · 2025-03-20T13:38:08Z

sdk/python/pulumi_esc_sdk/workspace_models.py

+    def from_json(self, dataJson: str):
+        data = json.loads(dataJson)
+        accounts: Dict[str, Account] = {}
+        accounts_dict = data.get("accounts")


this seems more natural to me, but what you have is fine

if "accounts" in data: for ...

Also, should we take into account if the creds file is in the wrong format? I assume this will throw an ambiguous attribute error, so if we can have a helpful error that might be better. (do we give a helpful error in pulumi/pulumi for the equivalent?)

Ty, I've no idea what's natural or not in python 😅

Hmm, creds files are not meant to be edited by hand, so I think it's fine to just throw an ambiguous "unexpected error". But I think it might be worth adding a test that makes sure that's what happens and SDK doesn't break.

I would prefer a clearer error but not a huge deal

In most scenarios it won't throw an error at all, just return None, it's only if you screw us the schema in very specific ways (like for example adding "accounts" key, but making it a string and not an object). I just think it would be overkill to try to catch all potential errors on bad deserializations

seanyeh · 2025-03-20T13:39:05Z

sdk/python/pulumi_esc_sdk/workspace.py

+        print(f"An unexpected error occured: {e}")
+        return None
+
+def get_current_account(shared) -> tuple[Account, str]:


what is shared for?

If Shared is true, we would ignore the ESC override and use the account selected in main credentials file - same thing as with the path method, not strictly necessary in this implementation, I was just keeping it as in the CLI.

if we're not using it, i would opt to remove it. but if you want to keep it to match the CLI, i would at least document it

Adding workspace/account logic for python

cd96e03

IaroslavTitov force-pushed the iaro/cli branch from af3086a to cd96e03 Compare March 19, 2025 18:28

IaroslavTitov requested review from seanyeh and komalali March 19, 2025 19:03

IaroslavTitov marked this pull request as ready for review March 19, 2025 19:04

cleverguy25 reviewed Mar 19, 2025

View reviewed changes

seanyeh reviewed Mar 19, 2025

View reviewed changes

Following up on comments

0e2f959

IaroslavTitov requested review from seanyeh and cleverguy25 March 19, 2025 21:41

komalali reviewed Mar 19, 2025

View reviewed changes

Fixing up bad syntax based on comments

e477f0d

komalali approved these changes Mar 19, 2025

View reviewed changes

seanyeh reviewed Mar 20, 2025

View reviewed changes

IaroslavTitov merged commit 64cf557 into main Mar 20, 2025
8 checks passed

This was referenced Mar 20, 2025

Fixing up missed comments on the python port #82

Merged

Porting account functionality for TS SDK #86

Open

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Adding workspace/account logic for python #81

Adding workspace/account logic for python #81

IaroslavTitov commented Mar 19, 2025 •

edited

Loading

cleverguy25 Mar 19, 2025

IaroslavTitov Mar 19, 2025

seanyeh Mar 19, 2025

komalali Mar 19, 2025

komalali Mar 19, 2025

IaroslavTitov Mar 19, 2025

komalali Mar 19, 2025

komalali Mar 19, 2025

IaroslavTitov Mar 19, 2025

komalali Mar 19, 2025

komalali Mar 19, 2025

komalali Mar 19, 2025

komalali Mar 19, 2025

komalali Mar 19, 2025

komalali Mar 19, 2025

komalali Mar 19, 2025

IaroslavTitov Mar 19, 2025

seanyeh left a comment

seanyeh Mar 20, 2025

IaroslavTitov Mar 20, 2025

seanyeh Mar 20, 2025

seanyeh Mar 20, 2025

IaroslavTitov Mar 20, 2025

seanyeh Mar 20, 2025

IaroslavTitov Mar 20, 2025

seanyeh Mar 20, 2025 •

edited

Loading

IaroslavTitov Mar 20, 2025

seanyeh Mar 20, 2025

IaroslavTitov Mar 20, 2025

seanyeh Mar 20, 2025

IaroslavTitov Mar 20, 2025

seanyeh Mar 20, 2025

	- Adding CLI Pulumi accounts loading logic into Python SDK
	- Adds support for reading credentials from disk to Python SDK

		@@ -0,0 +1,89 @@
		# Copyright 2025, Pulumi Corporation. All rights reserved.

		"""Recreating Pulumi workspace and account logic for python SDK."""

	"""Recreating Pulumi workspace and account logic for python SDK."""
	"""Pulumi workspace and account logic for python SDK."""

	return str(home_dir / ".pulumi")
	return str(home_dir.joinpath(".pulumi"))

	return str(pathlib.Path(dir) / "credentials.json")
	return str(pathlib.Path(dir).joinpath("credentials.json"))

		@@ -0,0 +1,41 @@
		# Copyright 2025, Pulumi Corporation. All rights reserved.

		"""Pulumi workspace models."""


		return str(home_dir.joinpath(".pulumi"))

		def get_pulumi_path(*elem: str) -> str:

Adding workspace/account logic for python #81

Adding workspace/account logic for python #81

Conversation

IaroslavTitov commented Mar 19, 2025 • edited Loading

Summary

Testing

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

seanyeh left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

seanyeh Mar 20, 2025 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

IaroslavTitov commented Mar 19, 2025 •

edited

Loading

seanyeh Mar 20, 2025 •

edited

Loading