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

feat(ui/ingest): ingestion form for Okta and AzureAD #9829

Merged
merged 7 commits into from
Mar 19, 2024

Conversation

gaurav2733
Copy link
Contributor

…a and AzureAD

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

@gaurav2733 gaurav2733 marked this pull request as draft February 13, 2024 16:59
@github-actions github-actions bot added product PR or Issue related to the DataHub UI/UX community-contribution PR or Issue raised by member(s) of DataHub Community labels Feb 13, 2024
@gaurav2733 gaurav2733 marked this pull request as ready for review February 14, 2024 13:19
Copy link
Collaborator

@chriscollins3456 chriscollins3456 left a comment

Choose a reason for hiding this comment

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

going to approve assuming that the fields we have here are what ingestion team would recommend but the frontend implementation looks good!

the main change before merging is to bring the copied function into a shared utility file. I'm approving anyways because I don't want to block once that change is made.

@@ -0,0 +1,174 @@
import { RecipeField, FieldType, setListValuesOnRecipe } from './common';

const validateURL = (fieldName) => {
Copy link
Collaborator

Choose a reason for hiding this comment

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

it looks like this function was copied from csv.ts - let's bring that function to a new shared utils.ts file and import it in the three recipes instead of copying it twice.

Then, it would be nice to have unit tests on that utility function

Copy link
Contributor Author

Choose a reason for hiding this comment

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

updated!

@anshbansal anshbansal changed the title feat(octa-azure-ingest-form): Add ingestion form to ingestion for Okt… feat(ui/ingest): ingestion form for Okta and AzureAD Feb 15, 2024
@asikowitz asikowitz removed the community-contribution PR or Issue raised by member(s) of DataHub Community label Feb 22, 2024
tooltip: 'Which Okta User Profile attribute to use as input to DataHub username mapping. Common values used are - login, email.',
type: FieldType.TEXT,
fieldPath: 'source.config.okta_profile_to_username_attr',
placeholder: 'usename',
Copy link
Collaborator

Choose a reason for hiding this comment

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

Change this to be email instead.

Add another option for okta_profile_to_username_regex

Move both to Advanced section

Copy link
Contributor Author

@gaurav2733 gaurav2733 Feb 27, 2024

Choose a reason for hiding this comment

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

"Can I change source.config.okta_profile_to_username_attr to source.config.email, or is it only the placeholder that needs to be changed? Additionally, should I move the okta_profile_to_username_regex in advance section or filter section?

import { validateURL } from '../../utils';
import { RecipeField, FieldType, setListValuesOnRecipe } from './common';

export const OKTA_DOMAIN_URL: RecipeField = {
Copy link
Collaborator

@anshbansal anshbansal Feb 26, 2024

Choose a reason for hiding this comment

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

Add option for stateful_ingestion. Only need to add stateful_ingestion.enabled in Advanced section.

Copy link
Collaborator

Choose a reason for hiding this comment

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

Add option for skip_users_without_a_group

import { validateURL } from '../../utils';
import { RecipeField, FieldType, setListValuesOnRecipe } from './common';

export const AZURE_CLIENT_ID: RecipeField = {
Copy link
Collaborator

Choose a reason for hiding this comment

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

Add option for stateful ingestion in advanced section

@anshbansal anshbansal merged commit 11f6ab6 into datahub-project:master Mar 19, 2024
36 checks passed
sleeperdeep pushed a commit to sleeperdeep/datahub that referenced this pull request Jun 25, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
product PR or Issue related to the DataHub UI/UX
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants