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

Shade protobuf for SingularityClient/SingularityBase #1642

Merged
merged 3 commits into from Nov 6, 2017

Conversation

Projects
None yet
3 participants
@ssalinas
Member

ssalinas commented Nov 2, 2017

Systems running protobuf 2.5 were having issues since mesos 1.x pulls in protobuf 2.6.1

@stevegutz

@ssalinas ssalinas added the hs_staging label Nov 2, 2017

@stevegutz

This comment has been minimized.

Show comment
Hide comment
@stevegutz

stevegutz Nov 2, 2017

Contributor

Looks reasonable to me. For things like protobuf its also common to add the version to the package name, eg com.hubspot.singularity.shaded.com.google.protobuf.twosixone or whatever.

Is shading just the client is enough? It doesn't leak into models or anything?

Contributor

stevegutz commented Nov 2, 2017

Looks reasonable to me. For things like protobuf its also common to add the version to the package name, eg com.hubspot.singularity.shaded.com.google.protobuf.twosixone or whatever.

Is shading just the client is enough? It doesn't leak into models or anything?

@ssalinas

This comment has been minimized.

Show comment
Hide comment
@ssalinas

ssalinas Nov 2, 2017

Member

Good point, might need it in SingularityBase as well. I can add that

Member

ssalinas commented Nov 2, 2017

Good point, might need it in SingularityBase as well. I can add that

@ssalinas ssalinas added this to the 0.19.0 milestone Nov 3, 2017

@ssalinas ssalinas changed the title from Shade protobuf for SingularityClient to Shade mesos for SingularityClient/SingularityBase Nov 3, 2017

@ssalinas ssalinas changed the title from Shade mesos for SingularityClient/SingularityBase to Shade protobuf for SingularityClient/SingularityBase Nov 3, 2017

ssalinas added some commits Nov 3, 2017

@ssalinas ssalinas added the hs_qa label Nov 3, 2017

@ssalinas

This comment has been minimized.

Show comment
Hide comment
@ssalinas

ssalinas Nov 3, 2017

Member

Ended up being easier to shade mesos completely here

Member

ssalinas commented Nov 3, 2017

Ended up being easier to shade mesos completely here

@baconmania

This comment has been minimized.

Show comment
Hide comment
@baconmania

baconmania Nov 3, 2017

Contributor

🚢

Contributor

baconmania commented Nov 3, 2017

🚢

@ssalinas ssalinas added the hs_stable label Nov 3, 2017

@ssalinas ssalinas modified the milestones: 0.19.0, 0.18.0 Nov 3, 2017

@ssalinas

This comment has been minimized.

Show comment
Hide comment
@ssalinas

ssalinas Nov 6, 2017

Member

There are a few more steps we need to take to solve the protos issues. This is a step in the right direction, so going to merge this and open an additional PR rather than continuing to work off this branch

Member

ssalinas commented Nov 6, 2017

There are a few more steps we need to take to solve the protos issues. This is a step in the right direction, so going to merge this and open an additional PR rather than continuing to work off this branch

@ssalinas ssalinas merged commit 7d24856 into master Nov 6, 2017

1 of 2 checks passed

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

@ssalinas ssalinas deleted the shade_protobuf branch Nov 6, 2017

<relocations>
<relocation>
<pattern>org.apache.mesos</pattern>
<shadedPattern>com.hubspot.singularity.shaded.org.apache.mesos.oneonetwo</shadedPattern>

This comment has been minimized.

@stevegutz

stevegutz Nov 6, 2017

Contributor

I don't think think that this will solve your problem. The issue isn't using an unshaded mesos, it's that mesos is using an unshaded protobuf that conflicts with what we want. So you want to rewrite the protobuf classes inside mesos, not mesos itself.

@stevegutz

stevegutz Nov 6, 2017

Contributor

I don't think think that this will solve your problem. The issue isn't using an unshaded mesos, it's that mesos is using an unshaded protobuf that conflicts with what we want. So you want to rewrite the protobuf classes inside mesos, not mesos itself.

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