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.

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

fix(vulnerabilities)/vulnerabilities_fixes_datahub #8075

Conversation

Sejal-NucleusTeq
Copy link
Contributor

Checklist

  • The PR conforms to DataHub's Contributing Guideline (particularly Commit Message Format)
  • Links to related issues (if applicable)
  • Tests for the changes have been added/updated (if applicable)
  • Docs related to the changes have been added/updated (if applicable). If a new feature has been added a Usage Guide has been added for the same.
  • For any breaking change/potential downtime/deprecation/big changes an entry has been made in Updating DataHub

@github-actions github-actions bot added the devops PR or Issue related to DataHub backend & deployment label May 18, 2023
@Sejal-NucleusTeq Sejal-NucleusTeq changed the title fix/vulnerabilities_fixes_datahub Fix/vulnerabilities_fixes_datahub May 18, 2023
@Sejal-NucleusTeq Sejal-NucleusTeq changed the title Fix/vulnerabilities_fixes_datahub fix/vulnerabilities_fixes_datahub May 18, 2023
@Sejal-NucleusTeq Sejal-NucleusTeq deleted the fix/vulnerabilities_fixes_datahub branch May 18, 2023 07:32
@Sejal-NucleusTeq Sejal-NucleusTeq restored the fix/vulnerabilities_fixes_datahub branch May 18, 2023 07:38
@Sejal-NucleusTeq Sejal-NucleusTeq changed the title fix/vulnerabilities_fixes_datahub fix(vulnerabilities)/vulnerabilities_fixes_datahub May 18, 2023
build.gradle Outdated
@@ -89,7 +89,7 @@ project.ext.externalDependency = [
'elasticSearchRest': 'org.elasticsearch.client:elasticsearch-rest-high-level-client:' + elasticsearchVersion,
'elasticSearchTransport': 'org.elasticsearch.client:transport:' + elasticsearchVersion,
'findbugsAnnotations': 'com.google.code.findbugs:annotations:3.0.1',
'graphqlJava': 'com.graphql-java:graphql-java:' + graphQLJavaVersion,
'graphqlJava': 'com.graphql-java:graphql-java:19.4',
Copy link
Collaborator

Choose a reason for hiding this comment

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

The version here should be incremented above from ext.graphQLJavaVersion = '19.0' to the 19.4 version.

build.gradle Outdated
'zookeeper': 'org.apache.zookeeper:zookeeper:3.4.14',
'wire': 'com.squareup.wire:wire-compiler:3.7.1',
'charle': 'com.charleskorn.kaml:kaml:0.53.0',
'jsonv': 'org.json:json:20230227',
Copy link
Collaborator

Choose a reason for hiding this comment

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

Looks like this is duplicated above 'json': 'org.json:json:20230227', is the same as this one. Please consolidate to one.

@@ -23,6 +25,9 @@ dependencies {
exclude group: "org.apache.htrace", module: "htrace-core4"
exclude group: "org.eclipse.jetty", module: "jetty-util"
exclude group: "org.apache.hadoop.thirdparty", module: "hadoop-shaded-protobuf_3_7"
exclude group: "com.charleskorn.kaml",module:"kaml"
Copy link
Collaborator

Choose a reason for hiding this comment

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

Couple of linting issues here, please add a space after the , to match the others. Thanks!


configurations.all{
exclude group: "commons-io",module:"commons-io"
Copy link
Collaborator

Choose a reason for hiding this comment

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

Similar to above linting command, please add a space after the comma.

}

configurations.all{
exclude group: "com.charleskorn.kaml",module:"kaml"
Copy link
Collaborator

Choose a reason for hiding this comment

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

linting

implementation externalDependency.charle
}
configurations.all{
exclude group: "com.charleskorn.kaml",module:"kaml"
Copy link
Collaborator

Choose a reason for hiding this comment

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

linting

@david-leifker
Copy link
Collaborator

Overall this looks good, just a few minor updates. If you enable us to modify the branch I can fix up the minor issues, otherwise look forward to a slight revision. Thank you for this!

@david-leifker david-leifker self-assigned this May 18, 2023
@Sejal-NucleusTeq
Copy link
Contributor Author

Sejal-NucleusTeq commented May 19, 2023 via email

@david-leifker
Copy link
Collaborator

Some of these fixes may have already been introduced to master, I suspect this is the branch conflicts. If possible please review the conflicts so that if there are remaining changes, we can merge them. Thank you!

@shirshanka shirshanka added release-0.10.3 community-contribution PR or Issue raised by member(s) of DataHub Community labels May 22, 2023
@david-leifker david-leifker self-requested a review May 23, 2023 14:52
@Sejal-NucleusTeq
Copy link
Contributor Author

Sejal-NucleusTeq commented May 24, 2023 via email

@Sejal-NucleusTeq
Copy link
Contributor Author

Sejal-NucleusTeq commented May 24, 2023

Hello David-leifker
Now Conflicts are resolved , please have a look on it.

@david-leifker david-leifker changed the base branch from master to nt-nuodata-vulnerabilites_fixes_datahub June 7, 2023 18:32
@david-leifker david-leifker merged commit 6ca2653 into datahub-project:nt-nuodata-vulnerabilites_fixes_datahub Jun 7, 2023
@david-leifker
Copy link
Collaborator

Merged to alternative branch so I can make sure all the tests are passing for before merging. Plan is to merge from that branch. #8189

david-leifker added a commit that referenced this pull request Jun 7, 2023
Co-authored-by: Sejal-NucleusTeq <109514187+Sejal-NucleusTeq@users.noreply.github.com>
@Sejal-NucleusTeq
Copy link
Contributor Author

Okay

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
community-contribution PR or Issue raised by member(s) of DataHub Community devops PR or Issue related to DataHub backend & deployment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants