fix: deepcopy component output to avoid overriding previous outputs #3698
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
jordanrfrazier
requested review from
edwinjosechittilappilly and
ogabrielluiz
dosubot
bot
added
size:XS
github-actions
bot
added
bug
|
This pull request is automatically being deployed by Amplify Hosting (learn more). |
|
Also closes #3584 |
edwinjosechittilappilly
approved these changes
Sep 5, 2024
ogabrielluiz
approved these changes
Sep 5, 2024
- Update loop to iterate over `_outputs.values()` instead of `outputs` to ensure correct attribute setting.
- Updated methods to iterate over `_outputs.values()` instead of `outputs`. - Ensured consistent access to output values across various methods. - Fixed potential issues with output value resetting and state model building.
- Updated all references of `_outputs` to `_outputs_map` in `component.py` to improve clarity and consistency. - Adjusted related methods and attributes in `base.py` to align with the new naming convention.
- Moved `_reset_all_output_values` call to ensure outputs are reset after initialization. - Updated attribute access to use `_outputs_map` instead of `_outputs` for consistency.
dosubot
bot
added
size:M
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This was a bug that @edwinjosechittilappilly noticed in the linked issue. Outputs of subsequently created components were overriding outputs of previously created components of the same name - i.e. two Model outputs may interchange their
Outputs, regardless of where in the flow they were.This is due to how the class variable
Outputsis structured -- as class variables, it gives us this potential for cross-component leakage, but I have no better ideas at the moment on how to allow initializing the component with Outputs and then allowing for dynamic updates to that variable to provide theresult.Closes #3619