Skip to content

feat: implement option key (#529)#552

Merged
camilamaia merged 1 commit intoscanapi:mainfrom
uesleicarvalhoo:529
Jul 8, 2022
Merged

feat: implement option key (#529)#552
camilamaia merged 1 commit intoscanapi:mainfrom
uesleicarvalhoo:529

Conversation

@uesleicarvalhoo
Copy link
Contributor

@uesleicarvalhoo uesleicarvalhoo commented Jun 22, 2022

Description

Implement key options to request and endpoint nodes for allow keywords in to request performing call.
The avaliable keys are timeout and verify. If both has same options, request node is priorized.

Motivation behind this PR?

#529

What type of change is this?

Feature

Checklist

  • I have added a changelog entry / my PR does not need a new changelog entry. Instructions.
  • I have added/updated unit tests. Instructions.
  • New and existing unit tests pass locally with my changes. Instructions
  • I have self-documented code my changes by adding docstring(s) and comment(s). Instructions
  • Current PR does not significantly decrease the code coverage and docstring coverage.
  • My code follows the style guidelines of this project.
  • I have run ScanAPI locally and manually tested my changes. Instructions.
  • I have squashed my commits. Instructions.

Issue

Closes #529

@uesleicarvalhoo uesleicarvalhoo requested review from a team as code owners June 22, 2022 22:07
Copy link

@github-actions github-actions bot left a comment

Choose a reason for hiding this comment

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

Thank you for supporting ScanAPI, and congratulations on your first contribution! A project committer will shortly review your contribution.

In the mean time, if you haven't had a chance please skim over the First Pull Request Guide which all pull requests must adhere to.

We hope to see you around!

@github-actions github-actions bot added the First Contribution First contribution to the project. label Jun 22, 2022
@Pradhvan
Copy link
Member

@uesleicarvalhoo thanks for the PR ⭐

Two of the checks that are failing are:

  1. Changelog Reminder / Changelog Reminder (pull_request)
  • Since it's a new feature can you please add a change log to a changelog. Add an entry to CHANGELOG.md in your branch. That should fix this check.
  1. ScanAPI Examples / poke-api (pull_request)
 [FAILED] pokeapi::pokemon::list_all::response_time_is_under_half_second
          response.elapsed.total_seconds() < 0.5 is false

This is the test that is failing that requires GET call to pokeapi to be under 0.5 sec. @camilamaia can this be due a flaky tests ?

@Pradhvan
Copy link
Member

@astenstrasser @camilamaia the changes looks good to me. What do you folks think ?

@camilamaia
Copy link
Member

Hey @uesleicarvalhoo the PR is amazing! Thanks for it 🤩 I tested it locally and works like a charm.

Only two small considerations:

  • Why verify is not allowed on the request scope? I believe this should work exactly the same way as timeout works. Did I miss something?
  • What do you think about changing from option to options since it is a dict and we can send more than one option?

BTW, I've created already an issue for the website to update our doc once this is merged and released :)

@uesleicarvalhoo
Copy link
Contributor Author

Hi @camilamaia, thanks! 😁

I thought that all requests is to same serve, then was no sense enalbe verify to one and disable to another one, but maybe some scenarios it makes sense, i will be update it!

@uesleicarvalhoo
Copy link
Contributor Author

@camilamaia is up to date, can you check it?

@camilamaia camilamaia merged commit 1b913ea into scanapi:main Jul 8, 2022
@camilamaia
Copy link
Member

@uesleicarvalhoo Congrats on your first merged PR! 🌟 Thank you very much!

I am going to send you an invite to join the ScanAPI org on GitHub 🚀 We invite everyone that has contributed with a merged PR in any of our repositories. Here you can check our Contributing Guidelines so you can understand better how it works.

Check your email/GitHub notifications to find the invite. If you accept it, you will also be able to clone directly our repositories, without needing to fork them.

Welcome on board and once again, thank you! 🙇‍♀️

@camilamaia camilamaia mentioned this pull request Aug 11, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

First Contribution First contribution to the project.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

There are some option to set verify=False to requests?

3 participants