-
-
Notifications
You must be signed in to change notification settings - Fork 532
[19.0] [MIG] queue_job,test_queue_job: migrate to 19.0 #832
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
Conversation
|
@jaredkipe OK, - rebased. |
282d7ed to
47f79ce
Compare
1a2b95d to
9e9ad38
Compare
bf38068 to
9c70e72
Compare
|
Thanks @richard-willdooit ! There are failing tests and a few pre-commit issues. Your PR came first, but there is another PR by @tishmen which I have not looked at yet, which is green. How would you like to proceed? |
|
@sbidoul Let's just get it sorted between us - if the other is definitely closer, then go with it, and cherry pick from here. I have been running this version for a long time on 18.3 locally, so I know it is pretty much there. I have NO idea what those new pre-commit issues are :S I just grabbed the latest and re-based - I was 100% passing before that. The test failure I did not check, and thought I would if someone could point me in why the pre-commit config seems to be wrong |
9c70e72 to
7a1cca5
Compare
7a1cca5 to
41c239d
Compare
|
Mine is there! Other than the pre-commit "parameters" which I do NOT understand, because none of that is in my commit. If you can shed any light, even if you reject my PR in the end, I would appreciate it. |
|
@richard-willdooit I believe the checks are not passing since you are trying to use base.demo_user and the CI runner does not use the flag --with-demo. I've amended my tests in my PR to use a python defined user in the test files itself. |
|
@tishmen Yes - it took me a while to work that one, thanks. I am just very very puzzled about the pre-commit. |
|
@richard-willdooit It is most certainly complicated. Check out my last comment on #833 about the dependency issues. Let's wait on @sbidoul regarding which PR to pick as we need to have base before anything else. I believe my whole PR with all of the queue modules migration should be accepted, so we avoid duplicate and additional work on top of that. |
No description provided.