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

Add HTTP Basic Auth option to OAuthHandler #15729

Merged
merged 1 commit into from
Jan 10, 2025
Merged

Add HTTP Basic Auth option to OAuthHandler #15729

merged 1 commit into from
Jan 10, 2025
151 −25

Conversation

Awk34
Copy link
Contributor

@Awk34 Awk34 commented Oct 26, 2024

No description provided.

@Awk34 Awk34 marked this pull request as draft October 30, 2024 17:07
@itsankit-google itsankit-google added the build Triggers github actions build label Nov 5, 2024
@itsankit-google itsankit-google marked this pull request as ready for review November 5, 2024 10:59
itsankit-google
itsankit-google requested changes Nov 5, 2024
View reviewed changes
Copy link
Member

@itsankit-google itsankit-google left a comment

Choose a reason for hiding this comment

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

Please add some unit tests in OAuthServiceTest.

...etl/cdap-data-pipeline-base/src/main/java/io/cdap/cdap/datapipeline/oauth/OAuthProvider.java Show resolved Hide resolved
...etl/cdap-data-pipeline-base/src/main/java/io/cdap/cdap/datapipeline/oauth/OAuthProvider.java Outdated Show resolved Hide resolved
...tl/cdap-data-pipeline-base/src/main/java/io/cdap/cdap/datapipeline/service/OAuthHandler.java Outdated Show resolved Hide resolved
...tl/cdap-data-pipeline-base/src/main/java/io/cdap/cdap/datapipeline/service/OAuthHandler.java Outdated Show resolved Hide resolved
...tl/cdap-data-pipeline-base/src/main/java/io/cdap/cdap/datapipeline/service/OAuthHandler.java Outdated Show resolved Hide resolved
...tl/cdap-data-pipeline-base/src/main/java/io/cdap/cdap/datapipeline/service/OAuthHandler.java Outdated Show resolved Hide resolved
@Awk34 Awk34 mentioned this pull request Nov 13, 2024
Open
@Awk34 Awk34 force-pushed the patch-1 branch 2 times, most recently from f131aac to 955be5d Compare December 23, 2024 07:36
@Awk34 Awk34 force-pushed the patch-1 branch 2 times, most recently from a407346 to 0eb3c57 Compare January 6, 2025 18:30
itsankit-google
itsankit-google reviewed Jan 6, 2025
View reviewed changes
Copy link
Member

@itsankit-google itsankit-google left a comment •
edited
Loading

Choose a reason for hiding this comment

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

Please revert submodule cdap-ui changes.

Please also rebase your PR with develop.

...tl/cdap-data-pipeline-base/src/main/java/io/cdap/cdap/datapipeline/service/OAuthHandler.java Outdated Show resolved Hide resolved
@Awk34 Awk34 force-pushed the patch-1 branch 4 times, most recently from d317a24 to 73394af Compare January 7, 2025 00:14
@Awk34 Awk34 force-pushed the patch-1 branch 2 times, most recently from c7c47f0 to 40bf8c7 Compare January 7, 2025 21:21
@Awk34 Awk34 force-pushed the patch-1 branch 4 times, most recently from 3a972c6 to 1b7508b Compare January 9, 2025 03:20
itsankit-google
itsankit-google approved these changes Jan 9, 2025
View reviewed changes
Copy link
Member

@itsankit-google itsankit-google left a comment

Choose a reason for hiding this comment

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

one minor comment, otherwise LGTM.

@Awk34
Copy link
Contributor Author

Awk34 commented Jan 9, 2025

The build-sonar failure doesn't seem related to this PR:

Error: Failed to execute goal org.sonarsource.scanner.maven:sonar-maven-plugin:4.0.0.4121:sonar (default-cli) on project cdap: Project not found. Please check the 'sonar.projectKey' and 'sonar.organization' properties, the 'SONAR_TOKEN' environment variable, or contact the project administrator to check the permissions of the user the token belongs to -> [Help 1]

@itsankit-google itsankit-google merged commit c7478d1 into cdapio:develop Jan 10, 2025
4 of 5 checks passed
@itsankit-google itsankit-google mentioned this pull request Jan 10, 2025
Merged
@Awk34 Awk34 deleted the patch-1 branch January 10, 2025 17:16
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@itsankit-google itsankit-google itsankit-google approved these changes

@vsethi09 vsethi09 Awaiting requested review from vsethi09

Assignees
No one assigned
Labels
build Triggers github actions build
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@Awk34 @itsankit-google