Skip to content
This repository was archived by the owner on Nov 8, 2024. It is now read-only.

Hook files must be sorted by file names. #26

Merged
merged 1 commit into from
Dec 9, 2016

Conversation

geekbrother
Copy link
Contributor

Example

We have some hook files, they named for control hooks flow in case of using glob filepathes:

10-first.py
20-second.py
30-third.py

dredd.yml using glob path:

hookfiles:
        - /var/_hooks/*.py

At the Dredd start it will produce console output:
info: Found Hookfiles: 0=/_hooks/10-first.py, 1=/_hooks/20-second.py, 2=/_hooks/30-third.py
so we'll expect this order of hooks execution.

But python glob returns unordered files list. As a result hook files are executed unordered and we can't control hooks execution flow.

@geekbrother geekbrother changed the title Hook files need to be sorted by file names. Hook files must be sorted by file names. Nov 24, 2016
@honzajavorek
Copy link
Contributor

@geekbrother Thanks for the PR! I think this is a good idea, but I'm actually considering this should be directly a part of Dredd so the behavior is consistent for all hooks.

I'm even surprised Dredd doesn't evaluate the glob before passing it to the hooks handler, because glob implementations can differ between languages. If this functionality was shifted to Dredd, glob evaluation would happen there and hook handlers would get a resolved list of files, would that be a solution for you? Would you be willing to work on it?

@geekbrother
Copy link
Contributor Author

It would be great to add this directly to Dredd, but it needs to change dredd -> hook_language communication for each hook language :( It can be done in feature and I can do this by PRs to Dredd and hook_languages repos.
But for now, I think we need to merge this PR, because this problem as I've tested only Python related.

@w-vi
Copy link
Contributor

w-vi commented Dec 8, 2016

I'd merge this one and solve it generally later, what do you think @honzajavorek ?

@honzajavorek
Copy link
Contributor

I agree!

@honzajavorek
Copy link
Contributor

I filed this: apiaryio/dredd#680

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

Successfully merging this pull request may close these issues.

3 participants