-
Notifications
You must be signed in to change notification settings - Fork 168
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: extract response ids in postman collection #352
feat: extract response ids in postman collection #352
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM, but could you add the CI workflow as discussed in the issue? Please also update the documentation where applicable.
also, you should rebase onto |
adding a postman post-request script to add the policy-id of "asset-1" as a postman environment variable of request "Get Cached Catalogs" and reusing it in "Initiate Negotiation".
…e for scripts is <=200 and <300
9e2ce57
to
54d808c
Compare
8ec1179
to
9ae59ff
Compare
@@ -48,6 +48,9 @@ jobs: | |||
- name: "Setup Kubectl" | |||
uses: azure/setup-kubectl@v4 | |||
|
|||
- name: "Setup Terraform" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
why is this necessary?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
see comment on the main thread
with: | ||
node-version: 18 | ||
|
||
- name: "Install Newman" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
why is this needed?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
can newman be directly used and does not have to be installed first?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
i think so. I had to merge the PR now, because I needed the terraform thing in another fix. I'll take care of that
@paullatzelsperger I've added some tests for the collection which are executed in a new GitHub action workflow "newman_test". Also I had to add an additional terraform setup step in all workflow scrips, as the workflows were failing this morning with "terraform: command not found". I hope this is ok and does not break anything else. |
Here is one example of a failing test run, even though I did not change anything regarding this test: https://github.com/eclipse-edc/MinimumViableDataspace/actions/runs/11252232440/job/31284932435 |
huh? that is .... very interesting. stumbled across the same thing too. |
What this PR changes/adds
The functionality to carry over variables from previous responses to following request if applicable. This should make it easier for the user to execute the chain of requests necessary to download data from the other dataspace.
Why it does that
Postman pre- and post-request scripts were added to accomplish this behavior. Also a pre-request check for the respective requests is added to make sure the variables carried over are defined before executing the request
Further notes
Linked Issue(s)
Closes #351