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

Remove mesos dep from SingularityBase #1648

Merged
merged 6 commits into from Nov 17, 2017

Conversation

Projects
None yet
2 participants
@ssalinas
Member

ssalinas commented Nov 7, 2017

This works towards solving a problem that has plagued us for a while now. We've been storing the mesos protos objects as json in zk/sql. With each mesos update, these can change shape and cause us issues with (de)serialization. Additionally, the update to mesos 1 pulled in protobuf 2.6.1, causing issues with ObjectMappers and jackson in SingularityClient for anything running a different protobuf version.

This PRs aims to recreate POJOs for most of the important mesos task info inside of SingularityBase, with the result of removing any dependency on org.apache.mesos from SingularityBase and the SingularityClient.

All mesos operations are currently moved to a mesos utils module (might move to SingularityService if it is the only one using it).

This is part way done, MesosOfferObject is finished, but still need to finish the MesosTaskObejct

ssalinas added some commits Nov 6, 2017

@ssalinas

This comment has been minimized.

Show comment
Hide comment
@ssalinas

ssalinas Nov 8, 2017

Member

Changed up the strategy on this. I realized if we try and keep our own POJOs for the entire mesos protos, we fall victim to the same breakage for updates. However, we only use a very small subset of the fields, and just store the rest for reference.

So, I've made POJOs for the fields in the mesos protos that we use, and utilized jackson's AnyGetter/AnySetter for the rest of the fields. This way everything will still be stored in the json for possible future usage, but we don't need to explicitly update beyond the fields that we actually need. With this the dep on mesos and on protobuf are now gone from SingularityBase. I'll be adding a release note to docs on the fact that the new objects mimic the protos for the most part, but the remaining fields would need to be accessed in different ways (or just using objectMapper.convert to go back to protos)

Member

ssalinas commented Nov 8, 2017

Changed up the strategy on this. I realized if we try and keep our own POJOs for the entire mesos protos, we fall victim to the same breakage for updates. However, we only use a very small subset of the fields, and just store the rest for reference.

So, I've made POJOs for the fields in the mesos protos that we use, and utilized jackson's AnyGetter/AnySetter for the rest of the fields. This way everything will still be stored in the json for possible future usage, but we don't need to explicitly update beyond the fields that we actually need. With this the dep on mesos and on protobuf are now gone from SingularityBase. I'll be adding a release note to docs on the fact that the new objects mimic the protos for the most part, but the remaining fields would need to be accessed in different ways (or just using objectMapper.convert to go back to protos)

@ssalinas ssalinas changed the title from (WIP) remove mesos dep from SingularityBase to Remove mesos dep from SingularityBase Nov 15, 2017

@ssalinas ssalinas added this to the 0.18.0 milestone Nov 15, 2017

@darcatron

This comment has been minimized.

Show comment
Hide comment
@darcatron

darcatron Nov 16, 2017

Contributor

🚢

Contributor

darcatron commented Nov 16, 2017

🚢

@ssalinas ssalinas added the hs_stable label Nov 16, 2017

@ssalinas ssalinas merged commit 418374f into master Nov 17, 2017

2 checks passed

continuous-integration/travis-ci/pr The Travis CI build passed
Details
continuous-integration/travis-ci/push The Travis CI build passed
Details

@ssalinas ssalinas deleted the base_no_mesos branch Nov 17, 2017

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment