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

[proxy] refactor post_post action #775

Merged
merged 1 commit into from
Jun 20, 2018
Merged

Conversation

mikz
Copy link
Contributor

@mikz mikz commented Jun 19, 2018

Extracted from #774

Refactors proxy:post_action to ease changes in #774. Proxy post_action should receive a context and be called from just one point - the policy.

@mikz mikz requested a review from a team as a code owner June 19, 2018 15:13
local formatted_usage = format_usage(self.usage)
ngx.timer.at(0, post_action, self, cached_key, backend, formatted_usage, self.credentials, response_codes_data())

ngx.timer.at(0, handle_timer_post_action, self, timers, cached_key, backend, formatted_usage, credentials, response_codes_data())
Copy link
Contributor Author

Choose a reason for hiding this comment

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

@davidor looks like this is missing self.extra_params_backend_authrep

Copy link
Contributor

Choose a reason for hiding this comment

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

Good catch 👍

This means that the referrer filter policy does not work when using the experimental async reporting. I'll add a test and fix the issue.

@davidor
Copy link
Contributor

davidor commented Jun 19, 2018

@mikz The integration tests were failing because the detection of whether to skip the post action was not done correctly. I've pushed a commit with the fix.

ngx.log(ngx.INFO, '[async] skipping after action, no cached key')
return
Copy link
Contributor Author

@mikz mikz Jun 20, 2018

Choose a reason for hiding this comment

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

😂

edit: just laughing at my poor "refactoring" :)

@mikz mikz force-pushed the proxy-post-action-refactor branch from 286eb23 to 7de9e0d Compare June 20, 2018 07:42
if not using_post_action then
timers:post(1)
if not using_sync_post_action then
semaphore:post(1)
Copy link
Contributor Author

Choose a reason for hiding this comment

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

I think this is really important when designing the resty.concurrent.task.
It needs to release the semaphore before continuing with the handler (that might terminate the request and the processing with ngx.exit) (even though I'm not sure how that behaves when executed in a timer)

* better naming: sync/async reporting
* proxy:post_action
  - receives context object
  - no need for the `force` parameter
* no need to trigger post_action from rewrite phase
@mikz mikz force-pushed the proxy-post-action-refactor branch from 7de9e0d to 10529e9 Compare June 20, 2018 07:46
@mikz mikz changed the title [wip] refactor post_post action [proxy] refactor post_post action Jun 20, 2018
@davidor davidor merged commit 9170208 into master Jun 20, 2018
@davidor davidor deleted the proxy-post-action-refactor branch June 20, 2018 08:11
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants