Skip to content
This repository has been archived by the owner on May 23, 2024. It is now read-only.

little improvements #124

Merged
merged 3 commits into from
Feb 27, 2019
Merged

little improvements #124

merged 3 commits into from
Feb 27, 2019

Conversation

AlexanderZagaynov
Copy link
Contributor

avoid redundant redirects

Currently, base resource paths got requested without trailing slashes in addresses.
It leads so such lines in the logs:

INFO -- : get https://example.com/api/v2/config
INFO -- Status: 301
INFO -- : get https://example.com/api/v2/config/
INFO -- Status: 200
INFO -- : get https://example.com/api/v2/inventories
INFO -- Status: 301
INFO -- : get https://example.com/api/v2/inventories/
INFO -- Status: 200

This commit eliminates useless redirect hops.

unhide url of not found resource

Ease 404-related errors debugging.

@miq-bot
Copy link
Collaborator

miq-bot commented Feb 12, 2019

Checked commits AlexanderZagaynov/ansible_tower_client_ruby@319263f~...33ce058 with ruby 2.3.3, rubocop 0.52.1, haml-lint 0.20.0, and yamllint 1.10.0
6 files checked, 0 offenses detected
Everything looks fine. 👍

@AlexanderZagaynov
Copy link
Contributor Author

@bdunne @jameswnl please review

@jameswnl
Copy link
Contributor

@bdunne can you take a look?

@bdunne bdunne merged commit 5ec38ba into ansible:master Feb 27, 2019
@bdunne bdunne self-assigned this Feb 27, 2019
@jrafanie
Copy link
Contributor

@AlexanderZagaynov this change will force VCR cassettes to be manually sed/changed or we'll have to re-record them for the ansible provider. This is because we previously recorded gets for config which will now be config/ and likewise for all collections.

@AlexanderZagaynov
Copy link
Contributor Author

@jrafanie makes sense. I also noticed one more place with missing trailing slash:

JSON.parse(get("me").body).fetch_path("results", 0, "username")

@jrafanie
Copy link
Contributor

@AlexanderZagaynov did you want to get that change in before we merge/release so we can re-record or sed/awk the vcr cassettes only once?

@jrafanie
Copy link
Contributor

@AlexanderZagaynov Ruby 2.5 support doesn't need to happen today or tomorrow.. it can wait until we have all of our changes we want into the gem. If so, please open a PR so we can try releasing a gem version next week. Thanks!

jrafanie added a commit to jrafanie/manageiq-providers-ansible_tower that referenced this pull request Mar 21, 2019
Collections and many other requests now request a single api/v1/project/ instead of api/v1/project
and redirecting to api/v1/project/.  Cassettes need to be updated to
show the change in the requests to add the trailing slash but also to
remove the redirected request since that no longer happens.

The client updated to remove redirects here:
ansible/ansible_tower_client_ruby#124
@AlexanderZagaynov AlexanderZagaynov deleted the fixes branch March 22, 2019 09:52
@AlexanderZagaynov
Copy link
Contributor Author

AlexanderZagaynov commented Mar 22, 2019

did you want to get that change in before we merge/release so we can re-record or sed/awk the vcr cassettes only once?

if you mean that me/ redirect, I've prepared a new PR for this, mentioned above.

Ruby 2.5 support doesn't need to happen today or tomorrow.. it can wait until we have all of our changes we want into the gem. If so, please open a PR so we can try releasing a gem version next week. Thanks!

done :)

jrafanie added a commit to jrafanie/manageiq-providers-ansible_tower that referenced this pull request Mar 25, 2019
Collections and many other requests now request a single api/v1/project/ instead of api/v1/project
and redirecting to api/v1/project/.  Cassettes need to be updated to
show the change in the requests to add the trailing slash but also to
remove the redirected request since that no longer happens.

The client updated to remove redirects here:
ansible/ansible_tower_client_ruby#124
jrafanie added a commit to jrafanie/manageiq-providers-ansible_tower that referenced this pull request Mar 25, 2019
Collections and many other requests now request a single api/v1/project/
instead of api/v1/project and redirecting to api/v1/project/.  Cassettes
were updated to remove the no longer redirected request since it no longer happens.

The client updated to remove redirects here:
ansible/ansible_tower_client_ruby#124
jrafanie added a commit to jrafanie/manageiq-providers-ansible_tower that referenced this pull request Mar 28, 2019
Manual backport of
ManageIQ#165

Collections and many other requests now request a single api/v1/project/
instead of api/v1/project and redirecting to api/v1/project/.  Cassettes
were updated to remove the no longer redirected request since it no longer happens.

The client updated to remove redirects here:
ansible/ansible_tower_client_ruby#124
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants