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

Run Spotless and exclude Checkstyle on client module #1392

Merged
merged 1 commit into from
Oct 21, 2021
Merged

Run Spotless and exclude Checkstyle on client module #1392

merged 1 commit into from
Oct 21, 2021

Conversation

owaiskazi19
Copy link
Member

@owaiskazi19 owaiskazi19 commented Oct 20, 2021 •
edited
Loading

Signed-off-by: Owais Kazi owaiskazi19@gmail.com

Description

Applied Spotless to client module and excluded it from CheckStyle.

Issues Resolved

Part of #1362 and #974

Check List

  • New functionality includes testing.
    • All tests pass
  • New functionality has been documented.
    • New functionality has javadoc added
  • Commits are signed per the DCO using --signoff

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.
For more information on following Developer Certificate of Origin and signing off your commits, please check here.

@owaiskazi19 owaiskazi19 changed the title Run spotless and exclude checkstyle on client module Run Spotless and exclude Checkstyle on client module Oct 20, 2021
owaiskazi19
owaiskazi19 commented Oct 20, 2021
View reviewed changes
gradle/formatting.gradle
':client:rest-high-level',
':client:sniffer',
':client:test',
':client:transport',
Copy link
Member Author

Choose a reason for hiding this comment

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

Removed client module from excluded path to run Spotless

@opensearch-ci-bot
Copy link
Collaborator

Can one of the admins verify this patch?

owaiskazi19
owaiskazi19 commented Oct 20, 2021
View reviewed changes
buildSrc/src/main/resources/checkstyle_suppressions.xml
@@ -19,21 +19,13 @@

<!-- Exclude server to run checkstyle -->
<suppress files="server" checks="." />
<!-- Exclude client to run checkstyle -->
<suppress files="client" checks="." />
Copy link
Member Author

Choose a reason for hiding this comment

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

Excluded client module to run CheckStyle

Copy link
Contributor

Choose a reason for hiding this comment

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

The comment seems misleading. Should rather be "Excludes checkstyle run on client module" as that's what it means.

@opensearch-ci-bot
Copy link
Collaborator

✅   Gradle Wrapper Validation success 4c99f32367219cbad813699cb8250df92d01f793

@opensearch-ci-bot
Copy link
Collaborator

✅   DCO Check Passed 4c99f32367219cbad813699cb8250df92d01f793

@opensearch-ci-bot
Copy link
Collaborator

✅   Gradle Precommit success 4c99f32367219cbad813699cb8250df92d01f793

2 similar comments
@opensearch-ci-bot
Copy link
Collaborator

✅   Gradle Precommit success 4c99f32367219cbad813699cb8250df92d01f793

@opensearch-ci-bot
Copy link
Collaborator

✅   Gradle Precommit success 4c99f32367219cbad813699cb8250df92d01f793

@tlfeng
Copy link
Collaborator

tlfeng commented Oct 20, 2021

start gradle check

@opensearch-ci-bot
Copy link
Collaborator

✅   Gradle Check success 4c99f32367219cbad813699cb8250df92d01f793
Log 750

Reports 750

setiah
setiah approved these changes Oct 20, 2021
View reviewed changes
Copy link
Contributor

@setiah setiah left a comment

Choose a reason for hiding this comment

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

Minor comments

@owaiskazi19
Copy link
Member Author

owaiskazi19 commented Oct 20, 2021 •
edited
Loading

Minor comments

I will fix the minor comment in the next coming PR. We can merge this.

@nknize
Copy link
Collaborator

nknize commented Oct 21, 2021

Minor comments

I will fix the minor comment in the next coming PR. We can merge this.

Are you okay w/ addressing the comments in the next PR @setiah? We don't want to forget; so if you want them addressed here so we don't have to carry them to the next PR let us know. I'm fine either way.

@owaiskazi19
Copy link
Member Author

owaiskazi19 commented Oct 21, 2021 •
edited
Loading

Minor comments

I will fix the minor comment in the next coming PR. We can merge this.

Are you okay w/ addressing the comments in the next PR @setiah? We don't want to forget; so if you want them addressed here so we don't have to carry them to the next PR let us know. I'm fine either way.

I am running the gradle check on my local once it's done I'll push the changes.

@owaiskazi19
Run spotless and exclude checkstyle on client module
599b36f 
Signed-off-by: Owais Kazi <owaiskazi19@gmail.com>
@opensearch-ci-bot
Copy link
Collaborator

✅   DCO Check Passed 599b36f

@opensearch-ci-bot
Copy link
Collaborator

✅   Gradle Wrapper Validation success 599b36f

@opensearch-ci-bot
Copy link
Collaborator

✅   Gradle Precommit success 599b36f

@tlfeng
Copy link
Collaborator

tlfeng commented Oct 21, 2021

start gradle check

@opensearch-ci-bot
Copy link
Collaborator

❌   Gradle Check failure 599b36f
Log 764

Reports 764

@tlfeng
Copy link
Collaborator

tlfeng commented Oct 21, 2021

In Log 764:

REPRODUCE WITH: ./gradlew ':server:internalClusterTest' --tests "org.opensearch.cluster.ClusterHealthIT.testHealthOnMasterFailover" -Dtests.seed=561EB51B32060BE7 -Dtests.security.manager=true -Dtests.jvm.argline="-XX:TieredStopAtLevel=1" -Dtests.locale=und -Dtests.timezone=Europe/Zaporozhye -Druntime.java=17

org.opensearch.cluster.ClusterHealthIT > testHealthOnMasterFailover FAILED
    java.util.concurrent.ExecutionException: RemoteTransportException[[node_s7][127.0.0.1:38917][cluster:monitor/health]]; nested: MasterNotDiscoveredException[NodeClosedException[node closed {node_s0}{1BopzH55RueXq3zHEzB4JQ}{sv97dBHoR8qcs7y78AAIlg}{127.0.0.1}{127.0.0.1:35059}{imr}{shard_indexing_pressure_enabled=true}]]; nested: NodeClosedException[node closed {node_s0}{1BopzH55RueXq3zHEzB4JQ}{sv97dBHoR8qcs7y78AAIlg}{127.0.0.1}{127.0.0.1:35059}{imr}{shard_indexing_pressure_enabled=true}];

@owaiskazi19
Copy link
Member Author

owaiskazi19 commented Oct 21, 2021 •
edited
Loading

Weird. Not able to reproduce it on my local. @tlfeng can we start the gradle check again to see if the tests are flaky over here?

@tlfeng
Copy link
Collaborator

tlfeng commented Oct 21, 2021

The failure might caused by multiple gradle check running simultaneously in Jenkins..
start gradle check

@opensearch-ci-bot
Copy link
Collaborator

✅   Gradle Check success 599b36f
Log 768

Reports 768

@setiah setiah merged commit 33e70a9 into opensearch-project:main Oct 21, 2021
owaiskazi19 added a commit to owaiskazi19/OpenSearch that referenced this pull request Oct 21, 2021
@owaiskazi19
Run spotless and exclude checkstyle on client module opensearch-proje…
731224a 
…ct#1392

Signed-off-by: Owais Kazi <owaiskazi19@gmail.com>
@owaiskazi19 owaiskazi19 mentioned this pull request Oct 21, 2021
Merged
5 tasks
dblock pushed a commit that referenced this pull request Oct 21, 2021
@owaiskazi19
Run spotless and exclude checkstyle on client module #1392 (#1414)
06a123b 
Signed-off-by: Owais Kazi <owaiskazi19@gmail.com>
@owaiskazi19 owaiskazi19 added the v1.2.0 Issues related to version 1.2.0 label Nov 11, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@setiah setiah setiah approved these changes

@saratvemulapalli saratvemulapalli Awaiting requested review from saratvemulapalli saratvemulapalli is a code owner

@VachaShah VachaShah Awaiting requested review from VachaShah VachaShah is a code owner

Assignees
No one assigned
Labels
v1.2.0 Issues related to version 1.2.0
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
5 participants
@owaiskazi19 @opensearch-ci-bot @tlfeng @nknize @setiah