-
Notifications
You must be signed in to change notification settings - Fork 167
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
Error when worker version doesn't match server #307 #327
Conversation
Thanks for your pull request. It looks like this may be your first contribution to a Google open source project (if not, look below for help). Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA). 📝 Please visit https://cla.developers.google.com/ to sign. Once you've signed (or fixed any issues), please reply here (e.g. What to do if you already signed the CLAIndividual signers
Corporate signers
|
@aarontp Hi! I've been looking at the code and I've got new questions
Sorry for the amount of questions, but I rather understand the whole thing so I can be useful by contributing to other issues too. : ) |
Hello: No worries, I think it's better to ask questions than get too far and have to change a bunch of code too, :)
|
@aarontp I keep being confuse with some things
Thanks for the help! |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hello:
I've made a couple comments on the PR, and here are some responses to your earlier questions:
- So the TurbiniaTask is created on the server, but it actually gets scheduled via a task scheduler to execute on a different worker. Since it gets created server-side, when you initially set the turbinia version in
TurbiniaTask.__init__()
, that will be the server version. Then, when the worker starts, it will have it's own turbinia.version variable that will be the Worker version, and that can be compared like you're doing in the run_wrapper(). The TurbiniaTaskResult() won't need to have the version as an attribute. - Yep, sounds good. I left comments on the code before I saw your comments, so I left a similar comment there.
TL;DR: I think you have the comparison correct, we just need to update it to return the results with the error (as you mention).
|
CLAs look good, thanks! |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM, thanks!
* Pin specific Celery/Kombu/Redis versions (google#305) * in TurbiniaTaskResult, input_evidence doesn't have to be a list (google#309) * input_evidence doesn't have to be a list * Fix exception * current pypi version of google-cloud-storage (1.13.0) requires google-cloud-core 0.28.1 (before the rename of google.cloud.iam (core) to google.api_core.iam (google#315) * Use a link to a "parent Evidence" instead of subclassing (google#296) * parent evidence * undo some simplifications for the sake of a simpler CL * Add some serialization * update docstring * Initialize attribute * set parent evidence if Evidence type is context dependant * don't pass parent_evidence at instantiation * undo linter stuff * comments * fix aim lib breaking tests * typo * Print version on start 3 (google#320) * Add files via upload * Delete turbiniactl.py * Delete turbiniactl.py * Add files via upload * Delete turbiniactl.py * Add files via upload * Update turbiniactl.py * Caps * Quick update to evidence docstrings (google#317) ... to disambiguate between _preprocess() and preprocess(). * Add Job filters (google#247) * Add job filters * fix docstrings. * update docstring * Get jobs filters working with new job manager * Refactor out FilterJobObjects into new method * Update YAPF * remove missed confict markers * Docstrings and renaming * Migrate job graph generator to use new manager (google#321) * Update Evidence local_path when it's saved (google#319) * Pin google-cloud-storage to 1.13.0 (google#326) Fixes google#325 Looks like google-cloud-storage was updated in: googleapis/google-cloud-python#6741 Which just got released as 1.13.1: https://pypi.org/project/google-cloud-storage/#history * Set image export to process all partitions (google#324) * Add --partitions all to image_export invocations * Fix typo * Explicitly set saved_paths to list (google#323) * Move version print after log level setup (google#322) * Move version print after log level setup * Remove extra whitespace * update the pystyle link (google#333) * Undefined name: Define 'unicode' in Python 3 (google#337) * Undefined name: Define 'unicode' in Python 3 __unicode()__ was removed in Python 3 because all __str__ are Unicode. [flake8](http://flake8.pycqa.org) testing of https://github.com/google/turbinia on Python 3.7.1 $ __flake8 . --count --select=E901,E999,F821,F822,F823 --show-source --statistics__ ``` ./tools/turbinia_job_graph.py:47:40: F821 undefined name 'unicode' parser.add_argument('filename', type=unicode, help='where to save the file') ^ 1 F821 undefined name 'unicode' 1 ``` * Placate PyLint * Added PSQ timeout to 1 week (google#336) * Error when worker version doesn't match server google#307 (google#327) * Added turbina_version to TurbinaTask * First approach * Changed to no rise error and return instead * Restored the run from run_wrapper * Changed format of strings * Changed words fixed line too long * bump version
Fixes #307
turbina.__version__
to theTurbinaTask
class as an attributeTurbiniaTask.run_wrapper()
beforeTurbiniaTask.run()
is ever ran