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

New plugin sanitize headers #25

Merged
merged 9 commits into from
Feb 21, 2019
Merged

New plugin sanitize headers #25

merged 9 commits into from
Feb 21, 2019

Conversation

thilp
Copy link
Member

@thilp thilp commented Feb 20, 2019

This is the next step in #11: converting the Sanitize Headers plugin from the old plugin architecture (with sequences of tasks as input/output) to the new one (with, in this particular case, a task as input/output).

Description

  • Added hooks for the OnTask, OnTaskSequence, OnScenario, and OnPythonProgram contracts in Transformer, so that the corresponding plugins receive their expected inputs.
  • Simplified implementation of "plugin contracts": instead of relying on function signatures (which requires sophisticated algorithms to deal with subtyping and the yet unstable typing API), use a basic decorator that annotates the plugin function with a private field. Contracts become elements of a Contract enum instead of being typing.Callable specs.
  • (minor) Improve error reporting when transformer is used from the CLI.

The new documentation about writing plugins is in the wiki.

Types of Changes

  • Refactor/improvements
  • Documentation / non-code

Tasks

List of tasks you will do to complete the PR:

  • What is described above.
  • Update the changelog.
  • Add functional tests. We can no longer rely on unit tests with so many moving parts.

This gets us closer to replacing Task2 with Task globally, and
simplifies the plugin's code.

Signed-off-by: Thibaut Le Page <[email protected]>
Signed-off-by: Thibaut Le Page <[email protected]>
transformer/test_locust.py Outdated Show resolved Hide resolved
transformer/plugins/contracts.py Outdated Show resolved Hide resolved
transformer/plugins/contracts.py Outdated Show resolved Hide resolved
Copy link
Contributor

@tortila tortila left a comment

Choose a reason for hiding this comment

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

Given the scope of this PR, I'm OK with adding the functional tests (that possibly cover more than just the current changes) in a separate PR. What do you think @thilp?

@thilp thilp merged commit 6519740 into master Feb 21, 2019
@thilp thilp deleted the new-plugin-sanitize-headers branch February 21, 2019 09:38
@tortila
Copy link
Contributor

tortila commented Feb 21, 2019

Created: #26 to address the outstanding task from this issue.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants