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

autoinstall: adjust x-make-view-request behavior #1540

Merged
merged 3 commits into from Jan 25, 2023

Conversation

dbungert
Copy link
Collaborator

@dbungert dbungert commented Jan 24, 2023

With autoinstall, a controller that was not interactive would decline
outright to provide the response to GET queries, with the notion that the
controller needed to be skipped. Limit this behavior to only when
x-make-view-request: yes is set, which lets the client be able to get
the real response, or the skip info, depending on need.

With autoinstall, a controller that was not interactive would decline
outright to provide the response to GET queries, with the notion that
the controller needed to be skipped.  Limit this behavior to only when
`x-make-view-request: yes` is set, which let's the client be able to get
the real response, or the skip info, depending on need.
Copy link
Collaborator

@mwhudson mwhudson left a comment

Choose a reason for hiding this comment

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

Thanks for this.

It's almost as if the client-server interaction was designed by a single developer without much review and in a hurry!


view_request_yes = await inst.get(
'/locale', headers={'x-make-view-request': 'yes'})
self.assertIsNone(view_request_yes)
Copy link
Collaborator

Choose a reason for hiding this comment

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

do you have any way to test the value of the x-skip header in the response?

Copy link
Collaborator

Choose a reason for hiding this comment

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

if the answer is "no" that's fine, having any tests >> having no tests!

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I debated doing so. I'll spend a little time on it and explore.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Implemented.

Copy link
Collaborator

@mwhudson mwhudson left a comment

Choose a reason for hiding this comment

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

thanks!

@dbungert dbungert merged commit d9e9cf3 into canonical:main Jan 25, 2023
@dbungert dbungert deleted the make-view-request-fix branch January 25, 2023 00:09
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
2 participants