-
Notifications
You must be signed in to change notification settings - Fork 2.9k
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(ingestion): Adds more advanced configurations for runtime debugging #8998
feat(ingestion): Adds more advanced configurations for runtime debugging #8998
Conversation
41f7852
to
154ed9b
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
looking good! I have some suggestions but nothing major on my end
datahub-web-react/src/app/ingest/source/IngestionSourceList.tsx
Outdated
Show resolved
Hide resolved
datahub-web-react/src/app/ingest/source/IngestionSourceList.tsx
Outdated
Show resolved
Hide resolved
datahub-web-react/src/app/ingest/source/IngestionSourceList.tsx
Outdated
Show resolved
Hide resolved
<Form.Item label={<Typography.Text strong>Extra Enviroment Variables</Typography.Text>}> | ||
<Typography.Paragraph> | ||
Advanced: Set extra environment variables to an ingestion execution | ||
</Typography.Paragraph> | ||
<Input | ||
data-testid="extra-args-input" | ||
placeholder='{"MY_CUSTOM_ENV": "my_custom_value2"}' | ||
value={retrieveExtraEnvs()} | ||
onChange={(event) => setExtraEnvs(event.target.value)} | ||
/> | ||
</Form.Item> | ||
<Form.Item label={<Typography.Text strong>Extra DataHub plugins</Typography.Text>}> | ||
<Typography.Paragraph> | ||
Advanced: Set extra DataHub plugins for an ingestion execution | ||
</Typography.Paragraph> | ||
<Input | ||
data-testid="extra-pip-plugin-input" | ||
placeholder='["debug"]' | ||
value={retrieveExtraDataHubPlugins()} | ||
onChange={(event) => setExtraDataHubPlugins(event.target.value)} | ||
/> | ||
</Form.Item> | ||
<Form.Item label={<Typography.Text strong>Extra Pip Libraries</Typography.Text>}> | ||
<Typography.Paragraph> | ||
Advanced: Add extra pip libraries for an ingestion execution | ||
</Typography.Paragraph> | ||
<Input | ||
data-testid="extra-pip-reqs-input" | ||
placeholder='["sqlparse==0.4.3"]' | ||
value={retrieveExtraReqs()} | ||
onChange={(event) => setExtraReqs(event.target.value)} | ||
/> | ||
</Form.Item> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
so im trying to get better at enforcing small component because that keeps the frontend so much cleaner and easier to reason about. I'd move each of these inputs into their own new small file and component where in each of those the getter and setter logic can live. ie. retrieveExtraReqs
and setExtraReqs
can live in its own component where you render the input for Extra Pip Libraries
metadata-models/src/main/pegasus/com/linkedin/ingestion/DataHubIngestionSourceInfo.pdl
Show resolved
Hide resolved
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
cool beans
Checklist
Adds advanced configurations for specifying environment variables, DataHub plugins and pip packages at runtime for managed ingestion.
This feature is specific to managed ingestion as it uses logic in datahub actions.
Tested manually.
Here is a screenshot of how it looks: