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

[automation/*] Add support for getting stack outputs using Workspace #6859

Merged
merged 10 commits into from
Apr 26, 2021

Conversation

vipentti
Copy link
Contributor

This adds the ability to get the stack outputs using Workspace without having to first create or select a stack.

This was inspired by @komalali's awesome self-service-platyform, specifically these lines.

Regarding the Python implementation, since I'm less familiar with Python:

I'm not sure what is the correct way to avoid circular dependencies, specifically regarding OutputMap, so I figured the simplest way would be to move OutputValue and OutputMap to a separate file _output.py and then import them where necessary.

I'm not sure if this considered a breaking change, which I would like to avoid. Based on my understanding the OutputValue and OutputMap are not part of the public interface since they are not mentioned in automation/__init__.py variable __all__.

@github-actions
Copy link

PR is now waiting for a maintainer to run the acceptance tests.

Note for the maintainer: To run the acceptance tests, please comment /run-acceptance-tests on the PR

@github-actions
Copy link

PR is now waiting for a maintainer to run the acceptance tests.

Note for the maintainer: To run the acceptance tests, please comment /run-acceptance-tests on the PR

@komalali komalali self-assigned this Apr 25, 2021
@komalali
Copy link
Member

Hey, @vipentti thanks for the PR!

Re: python - you're on the right track.
Specifically, we have named the files starting with an underscore as this is considered "private" in python. We've done this so we can change the file structure without breaking the public API. The public API is exposed through __all__ in automation/__init__.py. As for the fact that OutputValue and OutputMap are not included in __all__ in automation/__init__.py, that is an oversight and they should have been included.

All of that to say:

  • Moving OutputMap and OutputValue into _output.py is totally fine and non-breaking.
  • We should also include them in __all__ in automation/__init__.py (if you don't mind adding that on).

@komalali
Copy link
Member

/run-acceptance-tests

@github-actions
Copy link

Please view the results of the PR Build + Acceptance Tests Run Here

@komalali
Copy link
Member

Pylint errors in the test run are fixed in #6872

@komalali
Copy link
Member

komalali commented Apr 26, 2021

LGTM in general, but I would prefer naming it StackOutputs rather than Outputs when on Workspace

@vipentti
Copy link
Contributor Author

We should also include them in all in automation/init.py (if you don't mind adding that on).

I'll add OutputMap and OutputValue to __all__

LGTM in general, but I would prefer naming it StackOutputs rather than Outputs when on Workspace

This sounds reasonable. So for go, python and nodejs I'll rename Workspace.Outputs as Workspace.StackOutputs and for the dotnet version Workspace.GetStackOutputsAsync

@github-actions
Copy link

PR is now waiting for a maintainer to run the acceptance tests.

Note for the maintainer: To run the acceptance tests, please comment /run-acceptance-tests on the PR

@komalali
Copy link
Member

/run-acceptance-tests

@github-actions
Copy link

Please view the results of the PR Build + Acceptance Tests Run Here

1 similar comment
@github-actions
Copy link

Please view the results of the PR Build + Acceptance Tests Run Here

@komalali komalali merged commit daa6045 into pulumi:master Apr 26, 2021
abhinav pushed a commit to pulumi/pulumi-dotnet that referenced this pull request Jan 11, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants