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

Allow usage of response of previous request in the same integration #357

Merged
merged 3 commits into from
Sep 29, 2023

Conversation

GDay
Copy link
Member

@GDay GDay commented Sep 19, 2023

No description provided.

@coveralls
Copy link
Collaborator

coveralls commented Sep 19, 2023

Coverage Status

coverage: 93.488% (-0.02%) from 93.506% when pulling 1938f5f on usage-of-previous-result-in-integration into 7decda7 on master.

Copy link

@cscheng cscheng left a comment

Choose a reason for hiding this comment

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

Is there a reason why referencing by index is necessary? If you use unique names to store data for each execution, they can simple referenced by name, since the values are stored in the self.params variable. For example, if the first execute object store_data has {"primary_document_id": "id"}, the second one can have {"secondary_document_id": "id"}, and both can be used in the 3rd request.

@GDay
Copy link
Member Author

GDay commented Sep 26, 2023

@cscheng Besides the use here, I think this would also come in handy for getting data from some APIs (probably relatively rare, though).

For example, if the first execute object store_data

Note that store_data from the other PR saves the data from the new hire indefinitely in the system. Which is likely unnecessary for a bunch of manifests and could leave us with more sensitive data stored in the database than we might want.

I don't want the manifest to become larger than necessary: using additional variables for only locally referenced variables adds a few extra items to the manifest, while we could simply use indexes to get the correct data for an unlimited amount of execute items.

Unless you see a lot of downsides to this approach, then I prefer to go with the approach of this PR.

@cscheng
Copy link

cscheng commented Sep 29, 2023

Good point. I didn't think of that you would want to use a response value without saving it, but it might make sense in some cases to not store (private) data that might only be used once (for an e-mail). May be this is worth mentioning in the documentation near the store_data section?

@GDay GDay merged commit c246123 into master Sep 29, 2023
3 checks passed
@GDay GDay deleted the usage-of-previous-result-in-integration branch September 29, 2023 11:01
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.

3 participants