Skip to content
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.

Sign up for GitHub

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

Jump to bottom

Disable Jetty by default. #4964

Merged
merged 9 commits into from
Aug 14, 2019
Merged

Disable Jetty by default. #4964

merged 9 commits into from
Aug 14, 2019

Conversation

mitchdenny
Copy link
Contributor

No description provided.

@JonathanGiles
Copy link
Member

Can't wait to get this merged!

@mitchdenny
Copy link
Contributor Author

/azp run java - keyvault - ci

@azure-pipelines
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@JonathanGiles
Copy link
Member

Those CI times are super nice.

@mitchdenny
Copy link
Contributor Author

/azp run java - keyvault - ci.data

@azure-pipelines
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@mitchdenny
Copy link
Contributor Author

I think that this PR is ready to go. When it is merged all pipelines that include track 1 and track 2 libraries will continue to build as usual (including starting Jetty). Then for libraries which have no track 1 elements we can simply set the EnableJetty flag to false and Jetty will disappear from the build times.

For libraries that contain track 1 and track 2 libraries work needs to be done on a per library basis to split out the track 1 libraries into a separate build. This PR contains an example of how this is done for KeyVault (using profiles).

The template yml is setup so that Jetty will start unless it is specifically disabled, but the approach to the pom.service.xml is such that if EnableJetty is missing or false, only the track 2 components will build. So for the split out track 1 pipelines it is mandatory that EnableJetty setting is set to true so it all functions correctly.

@mitchdenny
Copy link
Contributor Author

Spoke too soon - looks like the track 1 build is failing. Not sure why.

@mitchdenny
Copy link
Contributor Author

OK - it looks like some recorded tests didn't come across in the move. Will need to go back and find them and put them in the right place.

@mitchdenny mitchdenny self-assigned this Aug 14, 2019
@mitchdenny mitchdenny added the EngSys This issue is impacting the engineering system. label Aug 14, 2019
@mitchdenny
Copy link
Contributor Author

/azp run java - keyvault - ci

@mitchdenny
Copy link
Contributor Author

/azp run java - keyvault - ci.data

@azure-pipelines
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

1 similar comment
@azure-pipelines
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

g2vinay
g2vinay reviewed Aug 14, 2019
View reviewed changes
sdk/keyvault/ci.yml Outdated Show resolved Hide resolved
sdk/keyvault/ci.yml Outdated Show resolved Hide resolved
@nemakam nemakam removed request for nemakam and yvgopal August 14, 2019 18:59
@mitchdenny mitchdenny merged commit 7711785 into Azure:master Aug 14, 2019
@mitchdenny mitchdenny deleted the disable-jetty-flag branch August 14, 2019 20:10
pull bot pushed a commit to test-repo-billy/azure-sdk-for-java that referenced this pull request Aug 14, 2019
@mitchdenny @pull
Disable Jetty by default. (Azure#4964)
5c28f74 
Adds a flag to the pipeline which controls whether it is a track 1 or track 2 build.
pull bot pushed a commit to test-repo-tih/azure-sdk-for-java that referenced this pull request Aug 14, 2019
@mitchdenny @pull
Disable Jetty by default. (Azure#4964)
ed8d111 
Adds a flag to the pipeline which controls whether it is a track 1 or track 2 build.
weshaggard
weshaggard reviewed Aug 14, 2019
View reviewed changes
Copy link
Member

@weshaggard weshaggard left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

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

@weshaggard weshaggard weshaggard left review comments

@g2vinay g2vinay g2vinay left review comments

@alzimmermsft alzimmermsft alzimmermsft approved these changes

@moderakh moderakh moderakh approved these changes

@danieljurek danieljurek Awaiting requested review from danieljurek

@samvaity samvaity Awaiting requested review from samvaity

@christopheranderson christopheranderson Awaiting requested review from christopheranderson

@conniey conniey Awaiting requested review from conniey

@jaschrep-msft jaschrep-msft Awaiting requested review from jaschrep-msft

@JonathanGiles JonathanGiles Awaiting requested review from JonathanGiles

@kushagraThapar kushagraThapar Awaiting requested review from kushagraThapar

@mssfang mssfang Awaiting requested review from mssfang

@rickle-msft rickle-msft Awaiting requested review from rickle-msft

@sima-zhu sima-zhu Awaiting requested review from sima-zhu

@srnagar srnagar Awaiting requested review from srnagar

@yaohaizh yaohaizh Awaiting requested review from yaohaizh

Assignees

@mitchdenny mitchdenny

Labels
EngSys This issue is impacting the engineering system.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
6 participants
@mitchdenny @JonathanGiles @g2vinay @weshaggard @moderakh @alzimmermsft