You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@superset.apache.org by "semohr (via GitHub)" <gi...@apache.org> on 2026/03/02 14:51:22 UTC
[PR] fix(assets): Support uploading tags using the assets import endpoint [superset]
semohr opened a new pull request, #38343:
URL: https://github.com/apache/superset/pull/38343
### SUMMARY
This pull request adds support for importing tags during the asset import process, conditional on the "TAGGING_SYSTEM" feature flag.
The changes are equivalent to the code in the dashboard import command see [here](https://github.com/semohr/superset/blob/985c3d12a175c3eac5c7a9c7462647122d36745b/superset/commands/dashboard/importers/v1/__init__.py#L207-L216).
### ADDITIONAL INFORMATION
- [x] Required feature flags: "TAGGING_SYSTEM"
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
Re: [PR] fix(assets): Support uploading tags using the assets import endpoint [superset]
Posted by "semohr (via GitHub)" <gi...@apache.org>.
semohr commented on code in PR #38343:
URL: https://github.com/apache/superset/pull/38343#discussion_r2883193223
##########
superset/commands/importers/v1/assets.py:
##########
@@ -87,7 +89,12 @@ def __init__(self, contents: dict[str, str], *args: Any, **kwargs: Any):
# pylint: disable=too-many-locals
@staticmethod
- def _import(configs: dict[str, Any], sparse: bool = False) -> None: # noqa: C901
+ def _import( # noqa: C901
+ configs: dict[str, Any],
Review Comment:
I dont think so :/ Just copied the same approach as used in the [dashboard importer command](https://github.com/apache/superset/blob/983b633972fde332921f63f28d04f8975f14459f/superset/commands/dashboard/importers/v1/__init__.py#L75).
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
Re: [PR] fix(assets): Support uploading tags using the assets import endpoint [superset]
Posted by "bito-code-review[bot] (via GitHub)" <gi...@apache.org>.
bito-code-review[bot] commented on PR #38343:
URL: https://github.com/apache/superset/pull/38343#issuecomment-4614178271
AI Code Review is in progress (usually takes 3 to 15 minutes unless it's a very large PR).
<details>
<summary>
Bito Usage Guide
</summary>
**Commands**
Type the following command in the pull request comment and save the comment.
- `/review` - Manually triggers a full AI review.
- `/pause` - Pauses automatic reviews on this pull request.
- `/resume` - Resumes automatic reviews.
- `/resolve` - Marks all Bito-posted review comments as resolved.
- `/abort` - Cancels all in-progress reviews.
Refer to the <a href="https://docs.bito.ai/bito-dev-agents/ai-code-review-agent/available-commands" target="_blank" rel="noopener noreferrer">documentation</a> for additional commands.
**Configuration**
This repository uses `Superset` You can customize the agent settings <a href="https://alpha.bito.ai/home/ai-agents/code-review-agent" target="_blank" rel="noopener noreferrer">here</a> or contact your Bito workspace admin at evan@preset.io.
**Documentation & Help**
- <a href="https://docs.bito.ai/bito-dev-agents/ai-code-review-agent/getting-started/install-run-using-bito-cloud/create-or-customize-an-agent-instance" target="_blank" rel="noopener noreferrer">Customize agent settings</a>
- <a href="https://docs.bito.ai/bito-dev-agents/ai-code-review-agent/implementing-custom-code-review-rules" target="_blank" rel="noopener noreferrer">Review rules</a>
- <a href="https://docs.bito.ai/bito-dev-agents/ai-code-review-agent" target="_blank" rel="noopener noreferrer">General documentation</a>
- <a href="https://docs.bito.ai/bito-dev-agents/ai-code-review-agent/faqs" target="_blank" rel="noopener noreferrer">FAQ</a>
</details>
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
Re: [PR] fix(assets): Support uploading tags using the assets import endpoint [superset]
Posted by "sfirke (via GitHub)" <gi...@apache.org>.
sfirke merged PR #38343:
URL: https://github.com/apache/superset/pull/38343
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
Re: [PR] fix(assets): Support uploading tags using the assets import endpoint [superset]
Posted by "EnxDev (via GitHub)" <gi...@apache.org>.
EnxDev commented on code in PR #38343:
URL: https://github.com/apache/superset/pull/38343#discussion_r3195372598
##########
superset/commands/export/assets.py:
##########
@@ -46,20 +48,34 @@ def run(self) -> Iterator[tuple[str, Callable[[], str]]]:
yield METADATA_FILE_NAME, lambda: yaml.safe_dump(metadata, sort_keys=False)
seen = {METADATA_FILE_NAME}
- commands = [
+ commands: list[type[ExportModelsCommand]] = [
ExportDatabasesCommand,
ExportDatasetsCommand,
ExportChartsCommand,
ExportDashboardsCommand,
ExportSavedQueriesCommand,
]
+ dashboard_ids: list[int | str] = []
+ chart_ids: list[int | str] = []
for command in commands:
ids = [model.id for model in command.dao.find_all()]
for file_name, file_content in command(ids, export_related=False).run():
if file_name not in seen:
yield file_name, file_content
seen.add(file_name)
+ if command == ExportDashboardsCommand:
+ dashboard_ids = ids
+ if command == ExportChartsCommand:
+ chart_ids = ids
Review Comment:
Could it make sense to use elif here?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
Re: [PR] fix(assets): Support uploading tags using the assets import endpoint [superset]
Posted by "rusackas (via GitHub)" <gi...@apache.org>.
rusackas commented on PR #38343:
URL: https://github.com/apache/superset/pull/38343#issuecomment-4299407000
Posted a comment on the other thread that you'll probably see, but thank you for this work, and we _still_ look forward to seeing it through.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
Re: [PR] fix(assets): Support uploading tags using the assets import endpoint [superset]
Posted by "codeant-ai-for-open-source[bot] (via GitHub)" <gi...@apache.org>.
codeant-ai-for-open-source[bot] commented on PR #38343:
URL: https://github.com/apache/superset/pull/38343#issuecomment-4046374651
## **Sequence Diagram**
This PR extends the assets workflow so tag metadata is included in exported bundles and reapplied during import. Tag restoration on import is gated by the TAGGING_SYSTEM feature flag.
```mermaid
sequenceDiagram
participant User
participant AssetsAPI
participant AssetsExport
participant AssetsImport
participant TagService
User->>AssetsAPI: Request asset export bundle
AssetsAPI->>AssetsExport: Export databases datasets charts dashboards queries
AssetsExport->>TagService: Export tags for chart and dashboard ids
TagService-->>AssetsExport: Tag metadata files
AssetsExport-->>User: Bundle includes assets and tags
User->>AssetsAPI: Upload asset bundle for import
AssetsAPI->>AssetsImport: Import assets from bundle
AssetsImport->>AssetsImport: Create charts and dashboards
alt TAGGING_SYSTEM enabled
AssetsImport->>TagService: Apply tags from bundle to imported assets
end
AssetsImport-->>User: Import completed with tags preserved
```
---
*Generated by [CodeAnt AI](https://codeant.ai)*
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
Re: [PR] fix(assets): Support uploading tags using the assets import endpoint [superset]
Posted by "bito-code-review[bot] (via GitHub)" <gi...@apache.org>.
bito-code-review[bot] commented on PR #38343:
URL: https://github.com/apache/superset/pull/38343#issuecomment-4154188680
AI Code Review is in progress (usually takes 3 to 15 minutes unless it's a very large PR).
<details>
<summary>
Bito Usage Guide
</summary>
**Commands**
Type the following command in the pull request comment and save the comment.
- `/review` - Manually triggers a full AI review.
- `/pause` - Pauses automatic reviews on this pull request.
- `/resume` - Resumes automatic reviews.
- `/resolve` - Marks all Bito-posted review comments as resolved.
- `/abort` - Cancels all in-progress reviews.
Refer to the <a href="https://docs.bito.ai/bito-dev-agents/ai-code-review-agent/available-commands" target="_blank" rel="noopener noreferrer">documentation</a> for additional commands.
**Configuration**
This repository uses `Superset` You can customize the agent settings <a href="https://alpha.bito.ai/home/ai-agents/code-review-agent" target="_blank" rel="noopener noreferrer">here</a> or contact your Bito workspace admin at evan@preset.io.
**Documentation & Help**
- <a href="https://docs.bito.ai/bito-dev-agents/ai-code-review-agent/getting-started/install-run-using-bito-cloud/create-or-customize-an-agent-instance" target="_blank" rel="noopener noreferrer">Customize agent settings</a>
- <a href="https://docs.bito.ai/bito-dev-agents/ai-code-review-agent/implementing-custom-code-review-rules" target="_blank" rel="noopener noreferrer">Review rules</a>
- <a href="https://docs.bito.ai/bito-dev-agents/ai-code-review-agent" target="_blank" rel="noopener noreferrer">General documentation</a>
- <a href="https://docs.bito.ai/bito-dev-agents/ai-code-review-agent/faqs" target="_blank" rel="noopener noreferrer">FAQ</a>
</details>
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
Re: [PR] fix(assets): Support uploading tags using the assets import endpoint [superset]
Posted by "bito-code-review[bot] (via GitHub)" <gi...@apache.org>.
bito-code-review[bot] commented on PR #38343:
URL: https://github.com/apache/superset/pull/38343#issuecomment-4046781593
AI Code Review is in progress (usually takes 3 to 15 minutes unless it's a very large PR).
<details>
<summary>
Bito Usage Guide
</summary>
**Commands**
Type the following command in the pull request comment and save the comment.
- `/review` - Manually triggers a full AI review.
- `/pause` - Pauses automatic reviews on this pull request.
- `/resume` - Resumes automatic reviews.
- `/resolve` - Marks all Bito-posted review comments as resolved.
- `/abort` - Cancels all in-progress reviews.
Refer to the <a href="https://docs.bito.ai/bito-dev-agents/ai-code-review-agent/available-commands" target="_blank" rel="noopener noreferrer">documentation</a> for additional commands.
**Configuration**
This repository uses `Superset` You can customize the agent settings <a href="https://alpha.bito.ai/home/ai-agents/code-review-agent" target="_blank" rel="noopener noreferrer">here</a> or contact your Bito workspace admin at evan@preset.io.
**Documentation & Help**
- <a href="https://docs.bito.ai/bito-dev-agents/ai-code-review-agent/getting-started/install-run-using-bito-cloud/create-or-customize-an-agent-instance" target="_blank" rel="noopener noreferrer">Customize agent settings</a>
- <a href="https://docs.bito.ai/bito-dev-agents/ai-code-review-agent/implementing-custom-code-review-rules" target="_blank" rel="noopener noreferrer">Review rules</a>
- <a href="https://docs.bito.ai/bito-dev-agents/ai-code-review-agent" target="_blank" rel="noopener noreferrer">General documentation</a>
- <a href="https://docs.bito.ai/bito-dev-agents/ai-code-review-agent/faqs" target="_blank" rel="noopener noreferrer">FAQ</a>
</details>
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
Re: [PR] fix(assets): Support uploading tags using the assets import endpoint [superset]
Posted by "sfirke (via GitHub)" <gi...@apache.org>.
sfirke commented on PR #38343:
URL: https://github.com/apache/superset/pull/38343#issuecomment-4129843269
Ugh that's annoying, sorry! I see you pushed changes and I restarted CI, let's see if that does anything?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
Re: [PR] fix(assets): Support uploading tags using the assets import endpoint [superset]
Posted by "EnxDev (via GitHub)" <gi...@apache.org>.
EnxDev commented on code in PR #38343:
URL: https://github.com/apache/superset/pull/38343#discussion_r3356325975
##########
superset/commands/export/assets.py:
##########
@@ -46,20 +48,34 @@ def run(self) -> Iterator[tuple[str, Callable[[], str]]]:
yield METADATA_FILE_NAME, lambda: yaml.safe_dump(metadata, sort_keys=False)
seen = {METADATA_FILE_NAME}
- commands = [
+ commands: list[type[ExportModelsCommand]] = [
ExportDatabasesCommand,
ExportDatasetsCommand,
ExportChartsCommand,
ExportDashboardsCommand,
ExportSavedQueriesCommand,
]
+ dashboard_ids: list[int | str] = []
+ chart_ids: list[int | str] = []
for command in commands:
ids = [model.id for model in command.dao.find_all()]
for file_name, file_content in command(ids, export_related=False).run():
if file_name not in seen:
yield file_name, file_content
seen.add(file_name)
+ if command == ExportDashboardsCommand:
+ dashboard_ids = ids
+ elif command == ExportChartsCommand:
+ chart_ids = ids
+
+ # FIXME: It would probably be better to align the tags export
+ # command with the other export commands
+ yield from ExportTagsCommand.export(
+ dashboard_ids=dashboard_ids,
+ chart_ids=chart_ids,
+ )
Review Comment:
@semohr how about this?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
Re: [PR] fix(assets): Support uploading tags using the assets import endpoint [superset]
Posted by "codecov[bot] (via GitHub)" <gi...@apache.org>.
codecov[bot] commented on PR #38343:
URL: https://github.com/apache/superset/pull/38343#issuecomment-3989791321
## [Codecov](https://app.codecov.io/gh/apache/superset/pull/38343?dropdown=coverage&src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) Report
:x: Patch coverage is `14.28571%` with `6 lines` in your changes missing coverage. Please review.
:white_check_mark: Project coverage is 54.54%. Comparing base ([`985c3d1`](https://app.codecov.io/gh/apache/superset/commit/985c3d12a175c3eac5c7a9c7462647122d36745b?dropdown=coverage&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache)) to head ([`23f146c`](https://app.codecov.io/gh/apache/superset/commit/23f146c84d75b0fe44c09a100285a361949175b1?dropdown=coverage&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache)).
:warning: Report is 16 commits behind head on master.
| [Files with missing lines](https://app.codecov.io/gh/apache/superset/pull/38343?dropdown=coverage&src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | Patch % | Lines |
|---|---|---|
| [superset/commands/importers/v1/assets.py](https://app.codecov.io/gh/apache/superset/pull/38343?src=pr&el=tree&filepath=superset%2Fcommands%2Fimporters%2Fv1%2Fassets.py&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache#diff-c3VwZXJzZXQvY29tbWFuZHMvaW1wb3J0ZXJzL3YxL2Fzc2V0cy5weQ==) | 14.28% | [6 Missing :warning: ](https://app.codecov.io/gh/apache/superset/pull/38343?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) |
:x: Your project check has failed because the head coverage (96.03%) is below the target coverage (100.00%). You can increase the head coverage or adjust the [target](https://docs.codecov.com/docs/commit-status#target) coverage.
<details><summary>Additional details and impacted files</summary>
```diff
@@ Coverage Diff @@
## master #38343 +/- ##
==========================================
- Coverage 64.27% 54.54% -9.73%
==========================================
Files 1810 2483 +673
Lines 71500 123355 +51855
Branches 22763 28640 +5877
==========================================
+ Hits 45954 67281 +21327
- Misses 25546 55758 +30212
- Partials 0 316 +316
```
| [Flag](https://app.codecov.io/gh/apache/superset/pull/38343/flags?src=pr&el=flags&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | Coverage Ξ | |
|---|---|---|
| [presto](https://app.codecov.io/gh/apache/superset/pull/38343/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `41.12% <14.28%> (?)` | |
| [python](https://app.codecov.io/gh/apache/superset/pull/38343/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `41.12% <14.28%> (?)` | |
Flags with carried forward coverage won't be shown. [Click here](https://docs.codecov.io/docs/carryforward-flags?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache#carryforward-flags-in-the-pull-request-comment) to find out more.
</details>
[:umbrella: View full report in Codecov by Sentry](https://app.codecov.io/gh/apache/superset/pull/38343?dropdown=coverage&src=pr&el=continue&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache).
:loudspeaker: Have feedback on the report? [Share it here](https://about.codecov.io/codecov-pr-comment-feedback/?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache).
<details><summary> :rocket: New features to boost your workflow: </summary>
- :snowflake: [Test Analytics](https://docs.codecov.com/docs/test-analytics): Detect flaky tests, report on failures, and find test suite problems.
- :package: [JS Bundle Analysis](https://docs.codecov.com/docs/javascript-bundle-analysis): Save yourself from yourself by tracking and limiting bundle sizes in JS merges.
</details>
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
Re: [PR] fix(assets): Support uploading tags using the assets import endpoint [superset]
Posted by "sfirke (via GitHub)" <gi...@apache.org>.
sfirke commented on PR #38343:
URL: https://github.com/apache/superset/pull/38343#issuecomment-4083662297
@semohr I see a CI check seems sad that there's not complete test coverage, if I'm understanding that correctly. Is that something you're able to address or explain?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
Re: [PR] fix(assets): Support uploading tags using the assets import endpoint [superset]
Posted by "bito-code-review[bot] (via GitHub)" <gi...@apache.org>.
bito-code-review[bot] commented on PR #38343:
URL: https://github.com/apache/superset/pull/38343#issuecomment-3984871422
AI Code Review is in progress (usually takes 3 to 15 minutes unless it's a very large PR).
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
Re: [PR] fix(assets): Support uploading tags using the assets import endpoint [superset]
Posted by "sfirke (via GitHub)" <gi...@apache.org>.
sfirke commented on PR #38343:
URL: https://github.com/apache/superset/pull/38343#issuecomment-4660663607
Thanks @semohr for this and your other contributions! And thanks @EnxDev for reviewing with care.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
Re: [PR] fix(assets): Support uploading tags using the assets import endpoint [superset]
Posted by "bito-code-review[bot] (via GitHub)" <gi...@apache.org>.
bito-code-review[bot] commented on PR #38343:
URL: https://github.com/apache/superset/pull/38343#issuecomment-3997411875
AI Code Review is in progress (usually takes 3 to 15 minutes unless it's a very large PR).
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
Re: [PR] fix(assets): Support uploading tags using the assets import endpoint [superset]
Posted by "semohr (via GitHub)" <gi...@apache.org>.
semohr commented on code in PR #38343:
URL: https://github.com/apache/superset/pull/38343#discussion_r2883610274
##########
superset/commands/importers/v1/assets.py:
##########
@@ -87,7 +89,12 @@ def __init__(self, contents: dict[str, str], *args: Any, **kwargs: Any):
# pylint: disable=too-many-locals
@staticmethod
- def _import(configs: dict[str, Any], sparse: bool = False) -> None: # noqa: C901
+ def _import( # noqa: C901
+ configs: dict[str, Any],
Review Comment:
Same applies to basically any import command. See e.g. also [charts](https://github.com/apache/superset/blob/983b633972fde332921f63f28d04f8975f14459f/superset/commands/chart/importers/v1/__init__.py#L56-L58) and [dataset](https://github.com/apache/superset/blob/983b633972fde332921f63f28d04f8975f14459f/superset/commands/dataset/importers/v1/__init__.py#L46-L48).
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
Re: [PR] fix(assets): Support uploading tags using the assets import endpoint [superset]
Posted by "bito-code-review[bot] (via GitHub)" <gi...@apache.org>.
bito-code-review[bot] commented on PR #38343:
URL: https://github.com/apache/superset/pull/38343#issuecomment-4112013704
AI Code Review is in progress (usually takes 3 to 15 minutes unless it's a very large PR).
<details>
<summary>
Bito Usage Guide
</summary>
**Commands**
Type the following command in the pull request comment and save the comment.
- `/review` - Manually triggers a full AI review.
- `/pause` - Pauses automatic reviews on this pull request.
- `/resume` - Resumes automatic reviews.
- `/resolve` - Marks all Bito-posted review comments as resolved.
- `/abort` - Cancels all in-progress reviews.
Refer to the <a href="https://docs.bito.ai/bito-dev-agents/ai-code-review-agent/available-commands" target="_blank" rel="noopener noreferrer">documentation</a> for additional commands.
**Configuration**
This repository uses `Superset` You can customize the agent settings <a href="https://alpha.bito.ai/home/ai-agents/code-review-agent" target="_blank" rel="noopener noreferrer">here</a> or contact your Bito workspace admin at evan@preset.io.
**Documentation & Help**
- <a href="https://docs.bito.ai/bito-dev-agents/ai-code-review-agent/getting-started/install-run-using-bito-cloud/create-or-customize-an-agent-instance" target="_blank" rel="noopener noreferrer">Customize agent settings</a>
- <a href="https://docs.bito.ai/bito-dev-agents/ai-code-review-agent/implementing-custom-code-review-rules" target="_blank" rel="noopener noreferrer">Review rules</a>
- <a href="https://docs.bito.ai/bito-dev-agents/ai-code-review-agent" target="_blank" rel="noopener noreferrer">General documentation</a>
- <a href="https://docs.bito.ai/bito-dev-agents/ai-code-review-agent/faqs" target="_blank" rel="noopener noreferrer">FAQ</a>
</details>
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
Re: [PR] fix(assets): Support uploading tags using the assets import endpoint [superset]
Posted by "semohr (via GitHub)" <gi...@apache.org>.
semohr commented on code in PR #38343:
URL: https://github.com/apache/superset/pull/38343#discussion_r3356446766
##########
superset/commands/export/assets.py:
##########
@@ -46,20 +48,34 @@ def run(self) -> Iterator[tuple[str, Callable[[], str]]]:
yield METADATA_FILE_NAME, lambda: yaml.safe_dump(metadata, sort_keys=False)
seen = {METADATA_FILE_NAME}
- commands = [
+ commands: list[type[ExportModelsCommand]] = [
ExportDatabasesCommand,
ExportDatasetsCommand,
ExportChartsCommand,
ExportDashboardsCommand,
ExportSavedQueriesCommand,
]
+ dashboard_ids: list[int | str] = []
+ chart_ids: list[int | str] = []
for command in commands:
ids = [model.id for model in command.dao.find_all()]
for file_name, file_content in command(ids, export_related=False).run():
if file_name not in seen:
yield file_name, file_content
seen.add(file_name)
+ if command == ExportDashboardsCommand:
+ dashboard_ids = ids
+ elif command == ExportChartsCommand:
+ chart_ids = ids
+
+ # FIXME: It would probably be better to align the tags export
+ # command with the other export commands
+ yield from ExportTagsCommand.export(
+ dashboard_ids=dashboard_ids,
+ chart_ids=chart_ids,
+ )
Review Comment:
You mean the comment?
```
# FIXME: It would probably be better to align the tags export
# command with the other export commands
```
For some reason `ExportTagsCommand` does not extend from `ExportModelsCommand` otherwise one could have just added it to the list of command classes above.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
Re: [PR] fix(assets): Support uploading tags using the assets import endpoint [superset]
Posted by "rusackas (via GitHub)" <gi...@apache.org>.
rusackas commented on PR #38343:
URL: https://github.com/apache/superset/pull/38343#issuecomment-4318394357
Approving CI π€
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
Re: [PR] fix(assets): Support uploading tags using the assets import endpoint [superset]
Posted by "bito-code-review[bot] (via GitHub)" <gi...@apache.org>.
bito-code-review[bot] commented on PR #38343:
URL: https://github.com/apache/superset/pull/38343#issuecomment-4084562375
AI Code Review is in progress (usually takes 3 to 15 minutes unless it's a very large PR).
<details>
<summary>
Bito Usage Guide
</summary>
**Commands**
Type the following command in the pull request comment and save the comment.
- `/review` - Manually triggers a full AI review.
- `/pause` - Pauses automatic reviews on this pull request.
- `/resume` - Resumes automatic reviews.
- `/resolve` - Marks all Bito-posted review comments as resolved.
- `/abort` - Cancels all in-progress reviews.
Refer to the <a href="https://docs.bito.ai/bito-dev-agents/ai-code-review-agent/available-commands" target="_blank" rel="noopener noreferrer">documentation</a> for additional commands.
**Configuration**
This repository uses `Superset` You can customize the agent settings <a href="https://alpha.bito.ai/home/ai-agents/code-review-agent" target="_blank" rel="noopener noreferrer">here</a> or contact your Bito workspace admin at evan@preset.io.
**Documentation & Help**
- <a href="https://docs.bito.ai/bito-dev-agents/ai-code-review-agent/getting-started/install-run-using-bito-cloud/create-or-customize-an-agent-instance" target="_blank" rel="noopener noreferrer">Customize agent settings</a>
- <a href="https://docs.bito.ai/bito-dev-agents/ai-code-review-agent/implementing-custom-code-review-rules" target="_blank" rel="noopener noreferrer">Review rules</a>
- <a href="https://docs.bito.ai/bito-dev-agents/ai-code-review-agent" target="_blank" rel="noopener noreferrer">General documentation</a>
- <a href="https://docs.bito.ai/bito-dev-agents/ai-code-review-agent/faqs" target="_blank" rel="noopener noreferrer">FAQ</a>
</details>
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
Re: [PR] fix(assets): Support uploading tags using the assets import endpoint [superset]
Posted by "sfirke (via GitHub)" <gi...@apache.org>.
sfirke commented on PR #38343:
URL: https://github.com/apache/superset/pull/38343#issuecomment-4137084824
Hi @semohr - @hainenber has a fix PR here: https://github.com/apache/superset/pull/38885
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
Re: [PR] fix(assets): Support uploading tags using the assets import endpoint [superset]
Posted by "hainenber (via GitHub)" <gi...@apache.org>.
hainenber commented on PR #38343:
URL: https://github.com/apache/superset/pull/38343#issuecomment-4140613493
@semohr you don't have to worry about failed CodeCov check on `core-package-ts`. It is indeed about [TypeScript files in frontend](https://github.com/apache/superset/blob/master/.codecov.yml#L18-L23), not `test-suite` abbreviated.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
Re: [PR] fix(assets): Support uploading tags using the assets import endpoint [superset]
Posted by "bito-code-review[bot] (via GitHub)" <gi...@apache.org>.
bito-code-review[bot] commented on PR #38343:
URL: https://github.com/apache/superset/pull/38343#issuecomment-4662040770
**Bito Automatic Review Skipped β PR Already Merged**<br/><br/>Bito scheduled an automatic review for this pull request, but the review was skipped because this PR was merged before the review could be run.<br/>No action is needed if you didn't intend to review it. To get a review, you can type `/review` in a comment and save it
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
Re: [PR] fix(assets): Support uploading tags using the assets import endpoint [superset]
Posted by "EnxDev (via GitHub)" <gi...@apache.org>.
EnxDev commented on code in PR #38343:
URL: https://github.com/apache/superset/pull/38343#discussion_r2877206050
##########
superset/commands/importers/v1/assets.py:
##########
@@ -136,6 +138,17 @@ def _import(configs: dict[str, Any], sparse: bool = False) -> None: # noqa: C90
charts.append(chart)
chart_ids[str(chart.uuid)] = chart.id
+ # Handle tags using import_tag function
+ if feature_flag_manager.is_feature_enabled("TAGGING_SYSTEM"):
+ if target_tag_names := config.get("tags"):
+ import_tag(
+ target_tag_names,
+ {},
Review Comment:
Do you think we should add a description here? Otherwise, the tags might be created without one.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
Re: [PR] fix(assets): Support uploading tags using the assets import endpoint [superset]
Posted by "bito-code-review[bot] (via GitHub)" <gi...@apache.org>.
bito-code-review[bot] commented on PR #38343:
URL: https://github.com/apache/superset/pull/38343#issuecomment-3990325684
AI Code Review is in progress (usually takes 3 to 15 minutes unless it's a very large PR).
<details>
<summary>
Bito Usage Guide
</summary>
**Commands**
Type the following command in the pull request comment and save the comment.
- `/review` - Manually triggers a full AI review.
- `/pause` - Pauses automatic reviews on this pull request.
- `/resume` - Resumes automatic reviews.
- `/resolve` - Marks all Bito-posted review comments as resolved.
- `/abort` - Cancels all in-progress reviews.
Refer to the <a href="https://docs.bito.ai/bito-dev-agents/ai-code-review-agent/available-commands" target="_blank" rel="noopener noreferrer">documentation</a> for additional commands.
**Configuration**
This repository uses `Superset` You can customize the agent settings <a href="https://alpha.bito.ai/home/ai-agents/code-review-agent" target="_blank" rel="noopener noreferrer">here</a> or contact your Bito workspace admin at evan@preset.io.
**Documentation & Help**
- <a href="https://docs.bito.ai/bito-dev-agents/ai-code-review-agent/getting-started/install-run-using-bito-cloud/create-or-customize-an-agent-instance" target="_blank" rel="noopener noreferrer">Customize agent settings</a>
- <a href="https://docs.bito.ai/bito-dev-agents/ai-code-review-agent/implementing-custom-code-review-rules" target="_blank" rel="noopener noreferrer">Review rules</a>
- <a href="https://docs.bito.ai/bito-dev-agents/ai-code-review-agent" target="_blank" rel="noopener noreferrer">General documentation</a>
- <a href="https://docs.bito.ai/bito-dev-agents/ai-code-review-agent/faqs" target="_blank" rel="noopener noreferrer">FAQ</a>
</details>
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
Re: [PR] fix(assets): Support uploading tags using the assets import endpoint [superset]
Posted by "bito-code-review[bot] (via GitHub)" <gi...@apache.org>.
bito-code-review[bot] commented on PR #38343:
URL: https://github.com/apache/superset/pull/38343#issuecomment-3987561214
AI Code Review is in progress (usually takes 3 to 15 minutes unless it's a very large PR).
<details>
<summary>
Bito Usage Guide
</summary>
**Commands**
Type the following command in the pull request comment and save the comment.
- `/review` - Manually triggers a full AI review.
- `/pause` - Pauses automatic reviews on this pull request.
- `/resume` - Resumes automatic reviews.
- `/resolve` - Marks all Bito-posted review comments as resolved.
- `/abort` - Cancels all in-progress reviews.
Refer to the <a href="https://docs.bito.ai/bito-dev-agents/ai-code-review-agent/available-commands" target="_blank" rel="noopener noreferrer">documentation</a> for additional commands.
**Configuration**
This repository uses `Superset` You can customize the agent settings <a href="https://alpha.bito.ai/home/ai-agents/code-review-agent" target="_blank" rel="noopener noreferrer">here</a> or contact your Bito workspace admin at evan@preset.io.
**Documentation & Help**
- <a href="https://docs.bito.ai/bito-dev-agents/ai-code-review-agent/getting-started/install-run-using-bito-cloud/create-or-customize-an-agent-instance" target="_blank" rel="noopener noreferrer">Customize agent settings</a>
- <a href="https://docs.bito.ai/bito-dev-agents/ai-code-review-agent/implementing-custom-code-review-rules" target="_blank" rel="noopener noreferrer">Review rules</a>
- <a href="https://docs.bito.ai/bito-dev-agents/ai-code-review-agent" target="_blank" rel="noopener noreferrer">General documentation</a>
- <a href="https://docs.bito.ai/bito-dev-agents/ai-code-review-agent/faqs" target="_blank" rel="noopener noreferrer">FAQ</a>
</details>
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
Re: [PR] fix(assets): Support uploading tags using the assets import endpoint [superset]
Posted by "bito-code-review[bot] (via GitHub)" <gi...@apache.org>.
bito-code-review[bot] commented on PR #38343:
URL: https://github.com/apache/superset/pull/38343#issuecomment-4305324916
AI Code Review is in progress (usually takes 3 to 15 minutes unless it's a very large PR).
<details>
<summary>
Bito Usage Guide
</summary>
**Commands**
Type the following command in the pull request comment and save the comment.
- `/review` - Manually triggers a full AI review.
- `/pause` - Pauses automatic reviews on this pull request.
- `/resume` - Resumes automatic reviews.
- `/resolve` - Marks all Bito-posted review comments as resolved.
- `/abort` - Cancels all in-progress reviews.
Refer to the <a href="https://docs.bito.ai/bito-dev-agents/ai-code-review-agent/available-commands" target="_blank" rel="noopener noreferrer">documentation</a> for additional commands.
**Configuration**
This repository uses `Superset` You can customize the agent settings <a href="https://alpha.bito.ai/home/ai-agents/code-review-agent" target="_blank" rel="noopener noreferrer">here</a> or contact your Bito workspace admin at evan@preset.io.
**Documentation & Help**
- <a href="https://docs.bito.ai/bito-dev-agents/ai-code-review-agent/getting-started/install-run-using-bito-cloud/create-or-customize-an-agent-instance" target="_blank" rel="noopener noreferrer">Customize agent settings</a>
- <a href="https://docs.bito.ai/bito-dev-agents/ai-code-review-agent/implementing-custom-code-review-rules" target="_blank" rel="noopener noreferrer">Review rules</a>
- <a href="https://docs.bito.ai/bito-dev-agents/ai-code-review-agent" target="_blank" rel="noopener noreferrer">General documentation</a>
- <a href="https://docs.bito.ai/bito-dev-agents/ai-code-review-agent/faqs" target="_blank" rel="noopener noreferrer">FAQ</a>
</details>
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
Re: [PR] fix(assets): Support uploading tags using the assets import endpoint [superset]
Posted by "semohr (via GitHub)" <gi...@apache.org>.
semohr commented on code in PR #38343:
URL: https://github.com/apache/superset/pull/38343#discussion_r3195680784
##########
superset/commands/export/assets.py:
##########
@@ -46,20 +48,34 @@ def run(self) -> Iterator[tuple[str, Callable[[], str]]]:
yield METADATA_FILE_NAME, lambda: yaml.safe_dump(metadata, sort_keys=False)
seen = {METADATA_FILE_NAME}
- commands = [
+ commands: list[type[ExportModelsCommand]] = [
ExportDatabasesCommand,
ExportDatasetsCommand,
ExportChartsCommand,
ExportDashboardsCommand,
ExportSavedQueriesCommand,
]
+ dashboard_ids: list[int | str] = []
+ chart_ids: list[int | str] = []
for command in commands:
ids = [model.id for model in command.dao.find_all()]
for file_name, file_content in command(ids, export_related=False).run():
if file_name not in seen:
yield file_name, file_content
seen.add(file_name)
+ if command == ExportDashboardsCommand:
+ dashboard_ids = ids
+ if command == ExportChartsCommand:
+ chart_ids = ids
Review Comment:
Good catch!
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
Re: [PR] fix(assets): Support uploading tags using the assets import endpoint [superset]
Posted by "semohr (via GitHub)" <gi...@apache.org>.
semohr commented on PR #38343:
URL: https://github.com/apache/superset/pull/38343#issuecomment-4623124827
> One optional nit: the FIXME about aligning the tags export with the other export commands could we either drop it or file a follow-up issue and reference it here, so it doesn't linger untracked? Not blocking.
Ofc let me create an issue for that :+1:
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
Re: [PR] fix(assets): Support uploading tags using the assets import endpoint [superset]
Posted by "semohr (via GitHub)" <gi...@apache.org>.
semohr commented on PR #38343:
URL: https://github.com/apache/superset/pull/38343#issuecomment-4089314284
The code coverage report is broken imo. I checked the files locally and what codecoverage reports is definitely not accurate.
```bash
pytest ./tests/unit_tests/commands/ --cov --cov-report html
```
<img width="780" height="1270" alt="image" src="https://github.com/user-attachments/assets/57ae8b25-0b53-4bab-8a8f-ec5b992da798" />
vs on codecoverage:
<img width="861" height="837" alt="image" src="https://github.com/user-attachments/assets/f32bb011-dc9f-4cc0-9ecc-222eb5e671f7" />
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
Re: [PR] fix(assets): Support uploading tags using the assets import endpoint [superset]
Posted by "semohr (via GitHub)" <gi...@apache.org>.
semohr commented on PR #38343:
URL: https://github.com/apache/superset/pull/38343#issuecomment-4130344310
Yeah a bit annoying. Seems like the problem persist tho. How do we want to continue? Anything else that is missing here?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
Re: [PR] fix(assets): Support uploading tags using the assets import endpoint [superset]
Posted by "bito-code-review[bot] (via GitHub)" <gi...@apache.org>.
bito-code-review[bot] commented on PR #38343:
URL: https://github.com/apache/superset/pull/38343#issuecomment-4176999333
AI Code Review is in progress (usually takes 3 to 15 minutes unless it's a very large PR).
<details>
<summary>
Bito Usage Guide
</summary>
**Commands**
Type the following command in the pull request comment and save the comment.
- `/review` - Manually triggers a full AI review.
- `/pause` - Pauses automatic reviews on this pull request.
- `/resume` - Resumes automatic reviews.
- `/resolve` - Marks all Bito-posted review comments as resolved.
- `/abort` - Cancels all in-progress reviews.
Refer to the <a href="https://docs.bito.ai/bito-dev-agents/ai-code-review-agent/available-commands" target="_blank" rel="noopener noreferrer">documentation</a> for additional commands.
**Configuration**
This repository uses `Superset` You can customize the agent settings <a href="https://alpha.bito.ai/home/ai-agents/code-review-agent" target="_blank" rel="noopener noreferrer">here</a> or contact your Bito workspace admin at evan@preset.io.
**Documentation & Help**
- <a href="https://docs.bito.ai/bito-dev-agents/ai-code-review-agent/getting-started/install-run-using-bito-cloud/create-or-customize-an-agent-instance" target="_blank" rel="noopener noreferrer">Customize agent settings</a>
- <a href="https://docs.bito.ai/bito-dev-agents/ai-code-review-agent/implementing-custom-code-review-rules" target="_blank" rel="noopener noreferrer">Review rules</a>
- <a href="https://docs.bito.ai/bito-dev-agents/ai-code-review-agent" target="_blank" rel="noopener noreferrer">General documentation</a>
- <a href="https://docs.bito.ai/bito-dev-agents/ai-code-review-agent/faqs" target="_blank" rel="noopener noreferrer">FAQ</a>
</details>
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
Re: [PR] fix(assets): Support uploading tags using the assets import endpoint [superset]
Posted by "semohr (via GitHub)" <gi...@apache.org>.
semohr commented on code in PR #38343:
URL: https://github.com/apache/superset/pull/38343#discussion_r2877370192
##########
superset/commands/importers/v1/assets.py:
##########
@@ -136,6 +138,17 @@ def _import(configs: dict[str, Any], sparse: bool = False) -> None: # noqa: C90
charts.append(chart)
chart_ids[str(chart.uuid)] = chart.id
+ # Handle tags using import_tag function
+ if feature_flag_manager.is_feature_enabled("TAGGING_SYSTEM"):
+ if target_tag_names := config.get("tags"):
+ import_tag(
+ target_tag_names,
+ {},
Review Comment:
Nvm figured it out. Added the contents (see [latest commit](https://github.com/apache/superset/pull/38343/commits/59c7ced7fc653dd56704648cfc87427221aa7c37))
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
Re: [PR] fix(assets): Support uploading tags using the assets import endpoint [superset]
Posted by "semohr (via GitHub)" <gi...@apache.org>.
semohr commented on code in PR #38343:
URL: https://github.com/apache/superset/pull/38343#discussion_r2874933803
##########
superset/commands/importers/v1/assets.py:
##########
@@ -136,6 +138,12 @@ def _import(configs: dict[str, Any], sparse: bool = False) -> None: # noqa: C90
charts.append(chart)
chart_ids[str(chart.uuid)] = chart.id
+ # Handle tags using import_tag function
+ if feature_flag_manager.is_feature_enabled("TAGGING_SYSTEM"):
+ if "tags" in config:
+ target_tag_names = config["tags"]
+ import_tag(target_tag_names, {}, chart.id, "chart", db.session)
Review Comment:
This makes absolutely no difference...
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
Re: [PR] fix(assets): Support uploading tags using the assets import endpoint [superset]
Posted by "sfirke (via GitHub)" <gi...@apache.org>.
sfirke commented on PR #38343:
URL: https://github.com/apache/superset/pull/38343#issuecomment-4199743633
@EnxDev can you re-review? The author seems to have addressed your comments and this is passing CI
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
Re: [PR] fix(assets): Support uploading tags using the assets import endpoint [superset]
Posted by "semohr (via GitHub)" <gi...@apache.org>.
semohr commented on PR #38343:
URL: https://github.com/apache/superset/pull/38343#issuecomment-4199736489
@sfirke It could indeed help. It looks like in your PR integration tests fail that (as I assume) compare the database state with an imported/exported version. If thatβs the case, and those tests rely on the assets import/export commands (which we have fixed here), then this PR might resolve those issues.
It is also very much possible tho that I'm missing some context here and there are more failures. Maybe you can try to rebase your PR onto this one and rerun the tests?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
Re: [PR] fix(assets): Support uploading tags using the assets import endpoint [superset]
Posted by "EnxDev (via GitHub)" <gi...@apache.org>.
EnxDev commented on PR #38343:
URL: https://github.com/apache/superset/pull/38343#issuecomment-3997411195
/review
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
Re: [PR] fix(assets): Support uploading tags using the assets import endpoint [superset]
Posted by "EnxDev (via GitHub)" <gi...@apache.org>.
EnxDev commented on code in PR #38343:
URL: https://github.com/apache/superset/pull/38343#discussion_r2883178047
##########
superset/commands/importers/v1/assets.py:
##########
@@ -87,7 +89,12 @@ def __init__(self, contents: dict[str, str], *args: Any, **kwargs: Any):
# pylint: disable=too-many-locals
@staticmethod
- def _import(configs: dict[str, Any], sparse: bool = False) -> None: # noqa: C901
+ def _import( # noqa: C901
+ configs: dict[str, Any],
Review Comment:
Do we have a better type instead of using `Any` here?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
Re: [PR] fix(assets): Support uploading tags using the assets import endpoint [superset]
Posted by "bito-code-review[bot] (via GitHub)" <gi...@apache.org>.
bito-code-review[bot] commented on PR #38343:
URL: https://github.com/apache/superset/pull/38343#issuecomment-4393890306
AI Code Review is in progress (usually takes 3 to 15 minutes unless it's a very large PR).
<details>
<summary>
Bito Usage Guide
</summary>
**Commands**
Type the following command in the pull request comment and save the comment.
- `/review` - Manually triggers a full AI review.
- `/pause` - Pauses automatic reviews on this pull request.
- `/resume` - Resumes automatic reviews.
- `/resolve` - Marks all Bito-posted review comments as resolved.
- `/abort` - Cancels all in-progress reviews.
Refer to the <a href="https://docs.bito.ai/bito-dev-agents/ai-code-review-agent/available-commands" target="_blank" rel="noopener noreferrer">documentation</a> for additional commands.
**Configuration**
This repository uses `Superset` You can customize the agent settings <a href="https://alpha.bito.ai/home/ai-agents/code-review-agent" target="_blank" rel="noopener noreferrer">here</a> or contact your Bito workspace admin at evan@preset.io.
**Documentation & Help**
- <a href="https://docs.bito.ai/bito-dev-agents/ai-code-review-agent/getting-started/install-run-using-bito-cloud/create-or-customize-an-agent-instance" target="_blank" rel="noopener noreferrer">Customize agent settings</a>
- <a href="https://docs.bito.ai/bito-dev-agents/ai-code-review-agent/implementing-custom-code-review-rules" target="_blank" rel="noopener noreferrer">Review rules</a>
- <a href="https://docs.bito.ai/bito-dev-agents/ai-code-review-agent" target="_blank" rel="noopener noreferrer">General documentation</a>
- <a href="https://docs.bito.ai/bito-dev-agents/ai-code-review-agent/faqs" target="_blank" rel="noopener noreferrer">FAQ</a>
</details>
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
Re: [PR] fix(assets): Support uploading tags using the assets import endpoint [superset]
Posted by "bito-code-review[bot] (via GitHub)" <gi...@apache.org>.
bito-code-review[bot] commented on PR #38343:
URL: https://github.com/apache/superset/pull/38343#issuecomment-4111312693
AI Code Review is in progress (usually takes 3 to 15 minutes unless it's a very large PR).
<details>
<summary>
Bito Usage Guide
</summary>
**Commands**
Type the following command in the pull request comment and save the comment.
- `/review` - Manually triggers a full AI review.
- `/pause` - Pauses automatic reviews on this pull request.
- `/resume` - Resumes automatic reviews.
- `/resolve` - Marks all Bito-posted review comments as resolved.
- `/abort` - Cancels all in-progress reviews.
Refer to the <a href="https://docs.bito.ai/bito-dev-agents/ai-code-review-agent/available-commands" target="_blank" rel="noopener noreferrer">documentation</a> for additional commands.
**Configuration**
This repository uses `Superset` You can customize the agent settings <a href="https://alpha.bito.ai/home/ai-agents/code-review-agent" target="_blank" rel="noopener noreferrer">here</a> or contact your Bito workspace admin at evan@preset.io.
**Documentation & Help**
- <a href="https://docs.bito.ai/bito-dev-agents/ai-code-review-agent/getting-started/install-run-using-bito-cloud/create-or-customize-an-agent-instance" target="_blank" rel="noopener noreferrer">Customize agent settings</a>
- <a href="https://docs.bito.ai/bito-dev-agents/ai-code-review-agent/implementing-custom-code-review-rules" target="_blank" rel="noopener noreferrer">Review rules</a>
- <a href="https://docs.bito.ai/bito-dev-agents/ai-code-review-agent" target="_blank" rel="noopener noreferrer">General documentation</a>
- <a href="https://docs.bito.ai/bito-dev-agents/ai-code-review-agent/faqs" target="_blank" rel="noopener noreferrer">FAQ</a>
</details>
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
Re: [PR] fix(assets): Support uploading tags using the assets import endpoint [superset]
Posted by "EnxDev (via GitHub)" <gi...@apache.org>.
EnxDev commented on PR #38343:
URL: https://github.com/apache/superset/pull/38343#issuecomment-4299358467
> @EnxDev can you re-review? The author seems to have addressed your comments and this is passing CI
Hi, Iβm really sorry for missing the last review round for this PR. I was on vacation, and afterward I had several priorities to take care of. Thanks a lot for the great work @semohr !
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
Re: [PR] fix(assets): Support uploading tags using the assets import endpoint [superset]
Posted by "semohr (via GitHub)" <gi...@apache.org>.
semohr commented on PR #38343:
URL: https://github.com/apache/superset/pull/38343#issuecomment-4290679837
@sfirke @EnxDev Seems a bit like the exact fix we proposed here was merged in #39495. How do you want us to continue?
That PR seems to be missing the export fix tho. Want me to rebase?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
Re: [PR] fix(assets): Support uploading tags using the assets import endpoint [superset]
Posted by "bito-code-review[bot] (via GitHub)" <gi...@apache.org>.
bito-code-review[bot] commented on PR #38343:
URL: https://github.com/apache/superset/pull/38343#issuecomment-4564120656
AI Code Review is in progress (usually takes 3 to 15 minutes unless it's a very large PR).
<details>
<summary>
Bito Usage Guide
</summary>
**Commands**
Type the following command in the pull request comment and save the comment.
- `/review` - Manually triggers a full AI review.
- `/pause` - Pauses automatic reviews on this pull request.
- `/resume` - Resumes automatic reviews.
- `/resolve` - Marks all Bito-posted review comments as resolved.
- `/abort` - Cancels all in-progress reviews.
Refer to the <a href="https://docs.bito.ai/bito-dev-agents/ai-code-review-agent/available-commands" target="_blank" rel="noopener noreferrer">documentation</a> for additional commands.
**Configuration**
This repository uses `Superset` You can customize the agent settings <a href="https://alpha.bito.ai/home/ai-agents/code-review-agent" target="_blank" rel="noopener noreferrer">here</a> or contact your Bito workspace admin at evan@preset.io.
**Documentation & Help**
- <a href="https://docs.bito.ai/bito-dev-agents/ai-code-review-agent/getting-started/install-run-using-bito-cloud/create-or-customize-an-agent-instance" target="_blank" rel="noopener noreferrer">Customize agent settings</a>
- <a href="https://docs.bito.ai/bito-dev-agents/ai-code-review-agent/implementing-custom-code-review-rules" target="_blank" rel="noopener noreferrer">Review rules</a>
- <a href="https://docs.bito.ai/bito-dev-agents/ai-code-review-agent" target="_blank" rel="noopener noreferrer">General documentation</a>
- <a href="https://docs.bito.ai/bito-dev-agents/ai-code-review-agent/faqs" target="_blank" rel="noopener noreferrer">FAQ</a>
</details>
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
Re: [PR] fix(assets): Support uploading tags using the assets import endpoint [superset]
Posted by "bito-code-review[bot] (via GitHub)" <gi...@apache.org>.
bito-code-review[bot] commented on PR #38343:
URL: https://github.com/apache/superset/pull/38343#issuecomment-4129997782
AI Code Review is in progress (usually takes 3 to 15 minutes unless it's a very large PR).
<details>
<summary>
Bito Usage Guide
</summary>
**Commands**
Type the following command in the pull request comment and save the comment.
- `/review` - Manually triggers a full AI review.
- `/pause` - Pauses automatic reviews on this pull request.
- `/resume` - Resumes automatic reviews.
- `/resolve` - Marks all Bito-posted review comments as resolved.
- `/abort` - Cancels all in-progress reviews.
Refer to the <a href="https://docs.bito.ai/bito-dev-agents/ai-code-review-agent/available-commands" target="_blank" rel="noopener noreferrer">documentation</a> for additional commands.
**Configuration**
This repository uses `Superset` You can customize the agent settings <a href="https://alpha.bito.ai/home/ai-agents/code-review-agent" target="_blank" rel="noopener noreferrer">here</a> or contact your Bito workspace admin at evan@preset.io.
**Documentation & Help**
- <a href="https://docs.bito.ai/bito-dev-agents/ai-code-review-agent/getting-started/install-run-using-bito-cloud/create-or-customize-an-agent-instance" target="_blank" rel="noopener noreferrer">Customize agent settings</a>
- <a href="https://docs.bito.ai/bito-dev-agents/ai-code-review-agent/implementing-custom-code-review-rules" target="_blank" rel="noopener noreferrer">Review rules</a>
- <a href="https://docs.bito.ai/bito-dev-agents/ai-code-review-agent" target="_blank" rel="noopener noreferrer">General documentation</a>
- <a href="https://docs.bito.ai/bito-dev-agents/ai-code-review-agent/faqs" target="_blank" rel="noopener noreferrer">FAQ</a>
</details>
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
Re: [PR] fix(assets): Support uploading tags using the assets import endpoint [superset]
Posted by "codeant-ai-for-open-source[bot] (via GitHub)" <gi...@apache.org>.
codeant-ai-for-open-source[bot] commented on code in PR #38343:
URL: https://github.com/apache/superset/pull/38343#discussion_r2872936997
##########
superset/commands/importers/v1/assets.py:
##########
@@ -136,6 +138,12 @@ def _import(configs: dict[str, Any], sparse: bool = False) -> None: # noqa: C90
charts.append(chart)
chart_ids[str(chart.uuid)] = chart.id
+ # Handle tags using import_tag function
+ if feature_flag_manager.is_feature_enabled("TAGGING_SYSTEM"):
+ if "tags" in config:
+ target_tag_names = config["tags"]
+ import_tag(target_tag_names, {}, chart.id, "chart", db.session)
Review Comment:
**Suggestion:** Missing null check for `config["tags"]` before calling `import_tag`. The schema defines `tags` with `allow_none=True`, meaning the value can be present in the config but set to `None`. Passing `None` to `import_tag` instead of a list will likely cause a TypeError or unexpected behavior when the function tries to iterate over the tags. [possible bug]
<details>
<summary><b>Severity Level:</b> Major β οΈ</summary>
```mdx
- β Asset import crashes when importing charts with null tags.
- β Bulk import operations fail if any chart has explicit null tags.
- β οΈ Database transaction rollback triggered, blocking all assets in batch.
```
</details>
```suggestion
# Handle tags using import_tag function
if feature_flag_manager.is_feature_enabled("TAGGING_SYSTEM"):
if config.get("tags"):
target_tag_names = config["tags"]
import_tag(target_tag_names, {}, chart.id, "chart", db.session)
```
<details>
<summary><b>Steps of Reproduction β
</b></summary>
```mdx
1. Enable TAGGING_SYSTEM feature flag in Superset configuration (required for PR
functionality).
2. Prepare asset import bundle containing a chart configuration file (e.g.,
`charts/my_chart.yaml`) with `tags: null` explicitly set (valid per ImportV1ChartSchema
line 1623 which allows null).
3. Trigger asset import via `ImportAssetsCommand.run()` at line 220, which calls
`_import()` at line 221.
4. In `_import()`, the chart processing loop (lines 133-145) encounters the chart config
with tags key present but None value.
5. Code at line 143 checks `if "tags" in config:` which evaluates to True when tags is
explicitly null.
6. Line 144 assigns `target_tag_names = None`, and line 145 calls `import_tag(None, {},
chart.id, "chart", db.session)`.
7. `import_tag` function (imported from `superset.commands.importers.v1.utils`) attempts
to iterate over None, raising TypeError and failing the entire asset import transaction.
```
</details>
<details>
<summary><b>Prompt for AI Agent π€ </b></summary>
```mdx
This is a comment left during a code review.
**Path:** superset/commands/importers/v1/assets.py
**Line:** 141:145
**Comment:**
*Possible Bug: Missing null check for `config["tags"]` before calling `import_tag`. The schema defines `tags` with `allow_none=True`, meaning the value can be present in the config but set to `None`. Passing `None` to `import_tag` instead of a list will likely cause a TypeError or unexpected behavior when the function tries to iterate over the tags.
Validate the correctness of the flagged issue. If correct, How can I resolve this? If you propose a fix, implement it and please make it concise.
```
</details>
<a href='https://app.codeant.ai/feedback?pr_url=https%3A%2F%2Fgithub.com%2Fapache%2Fsuperset%2Fpull%2F38343&comment_hash=5bd53b80469e3bbd442ccb7914122e78ca261746070be4b27e4d75c4a101e1c8&reaction=like'>π</a> | <a href='https://app.codeant.ai/feedback?pr_url=https%3A%2F%2Fgithub.com%2Fapache%2Fsuperset%2Fpull%2F38343&comment_hash=5bd53b80469e3bbd442ccb7914122e78ca261746070be4b27e4d75c4a101e1c8&reaction=dislike'>π</a>
##########
superset/commands/importers/v1/assets.py:
##########
@@ -154,6 +162,18 @@ def _import(configs: dict[str, Any], sparse: bool = False) -> None: # noqa: C90
}
dashboard_chart_ids.append(dashboard_chart_id)
+ # Handle tags using import_tag function
+ if feature_flag_manager.is_feature_enabled("TAGGING_SYSTEM"):
+ if "tags" in config:
+ target_tag_names = config["tags"]
+ import_tag(
+ target_tag_names,
+ {},
+ dashboard.id,
+ "dashboard",
+ db.session,
+ )
Review Comment:
**Suggestion:** Missing null check for `config["tags"]` before calling `import_tag`. The schema allows the `tags` field to be null (`allow_none=True`), so the code should verify that the value is not None before passing it to `import_tag`. Current code only checks if the key exists, which is insufficient when the value is explicitly set to null. [possible bug]
<details>
<summary><b>Severity Level:</b> Major β οΈ</summary>
```mdx
- β Dashboard import fails when tags field is explicitly null.
- β Complete asset import batch aborted due to unhandled TypeError.
- β οΈ Feature behind TAGGING_SYSTEM flag; affects tagged dashboard workflows.
```
</details>
```suggestion
# Handle tags using import_tag function
if feature_flag_manager.is_feature_enabled("TAGGING_SYSTEM"):
if config.get("tags"):
target_tag_names = config["tags"]
import_tag(
target_tag_names,
{},
dashboard.id,
"dashboard",
db.session,
)
```
<details>
<summary><b>Steps of Reproduction β
</b></summary>
```mdx
1. Enable TAGGING_SYSTEM feature flag in Superset configuration environment.
2. Prepare asset import with dashboard configuration (e.g.,
`dashboards/my_dashboard.yaml`) containing `tags: null` (valid per ImportV1DashboardSchema
line 519 which allows null).
3. Execute import via `ImportAssetsCommand` which invokes `_import()` static method at
line 128.
4. Dashboard processing loop (lines 148-184) processes the dashboard config.
5. At line 167, check `if "tags" in config:` returns True when tags key exists with null
value.
6. Line 168 assigns None to `target_tag_names`, lines 169-175 pass None to `import_tag()`.
7. `import_tag` fails when attempting to iterate over the None value, causing
ImportFailedError and rolling back the import transaction.
```
</details>
<details>
<summary><b>Prompt for AI Agent π€ </b></summary>
```mdx
This is a comment left during a code review.
**Path:** superset/commands/importers/v1/assets.py
**Line:** 165:175
**Comment:**
*Possible Bug: Missing null check for `config["tags"]` before calling `import_tag`. The schema allows the `tags` field to be null (`allow_none=True`), so the code should verify that the value is not None before passing it to `import_tag`. Current code only checks if the key exists, which is insufficient when the value is explicitly set to null.
Validate the correctness of the flagged issue. If correct, How can I resolve this? If you propose a fix, implement it and please make it concise.
```
</details>
<a href='https://app.codeant.ai/feedback?pr_url=https%3A%2F%2Fgithub.com%2Fapache%2Fsuperset%2Fpull%2F38343&comment_hash=a99dd13f557233cb2fbd03dfb345a9427ba292f62a3d3616ae09cca692cb52c9&reaction=like'>π</a> | <a href='https://app.codeant.ai/feedback?pr_url=https%3A%2F%2Fgithub.com%2Fapache%2Fsuperset%2Fpull%2F38343&comment_hash=a99dd13f557233cb2fbd03dfb345a9427ba292f62a3d3616ae09cca692cb52c9&reaction=dislike'>π</a>
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
Re: [PR] fix(assets): Support uploading tags using the assets import endpoint [superset]
Posted by "sfirke (via GitHub)" <gi...@apache.org>.
sfirke commented on PR #38343:
URL: https://github.com/apache/superset/pull/38343#issuecomment-4199663978
BTW @semohr I tried to set the value of `TAGGING_SYSTEM` feature flag to `True` and my PR failed checks I think because import/export tests failed. My PR is https://github.com/apache/superset/pull/37852, I wonder if yours will help with mine do you think?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
Re: [PR] fix(assets): Support uploading tags using the assets import endpoint [superset]
Posted by "netlify[bot] (via GitHub)" <gi...@apache.org>.
netlify[bot] commented on PR #38343:
URL: https://github.com/apache/superset/pull/38343#issuecomment-3987257879
### <span aria-hidden="true">β
</span> Deploy Preview for *superset-docs-preview* ready!
| Name | Link |
|:-:|------------------------|
|<span aria-hidden="true">π¨</span> Latest commit | 23f146c84d75b0fe44c09a100285a361949175b1 |
|<span aria-hidden="true">π</span> Latest deploy log | https://app.netlify.com/projects/superset-docs-preview/deploys/69a60ba809affe0008514e7e |
|<span aria-hidden="true">π</span> Deploy Preview | [https://deploy-preview-38343--superset-docs-preview.netlify.app](https://deploy-preview-38343--superset-docs-preview.netlify.app) |
|<span aria-hidden="true">π±</span> Preview on mobile | <details><summary> Toggle QR Code... </summary><br /><br /><br /><br />_Use your smartphone camera to open QR code link._</details> |
---
<!-- [superset-docs-preview Preview](https://deploy-preview-38343--superset-docs-preview.netlify.app) -->
_To edit notification comments on pull requests, go to your [Netlify project configuration](https://app.netlify.com/projects/superset-docs-preview/configuration/notifications#deploy-notifications)._
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
Re: [PR] fix(assets): Support uploading tags using the assets import endpoint [superset]
Posted by "semohr (via GitHub)" <gi...@apache.org>.
semohr commented on code in PR #38343:
URL: https://github.com/apache/superset/pull/38343#discussion_r2877246410
##########
superset/commands/importers/v1/assets.py:
##########
@@ -136,6 +138,17 @@ def _import(configs: dict[str, Any], sparse: bool = False) -> None: # noqa: C90
charts.append(chart)
chart_ids[str(chart.uuid)] = chart.id
+ # Handle tags using import_tag function
+ if feature_flag_manager.is_feature_enabled("TAGGING_SYSTEM"):
+ if target_tag_names := config.get("tags"):
+ import_tag(
+ target_tag_names,
+ {},
Review Comment:
Good point! Any idea where and how we can get the descriptions?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
Re: [PR] fix(assets): Support uploading tags using the assets import endpoint [superset]
Posted by "bito-code-review[bot] (via GitHub)" <gi...@apache.org>.
bito-code-review[bot] commented on PR #38343:
URL: https://github.com/apache/superset/pull/38343#issuecomment-3990934978
AI Code Review is in progress (usually takes 3 to 15 minutes unless it's a very large PR).
<details>
<summary>
Bito Usage Guide
</summary>
**Commands**
Type the following command in the pull request comment and save the comment.
- `/review` - Manually triggers a full AI review.
- `/pause` - Pauses automatic reviews on this pull request.
- `/resume` - Resumes automatic reviews.
- `/resolve` - Marks all Bito-posted review comments as resolved.
- `/abort` - Cancels all in-progress reviews.
Refer to the <a href="https://docs.bito.ai/bito-dev-agents/ai-code-review-agent/available-commands" target="_blank" rel="noopener noreferrer">documentation</a> for additional commands.
**Configuration**
This repository uses `Superset` You can customize the agent settings <a href="https://alpha.bito.ai/home/ai-agents/code-review-agent" target="_blank" rel="noopener noreferrer">here</a> or contact your Bito workspace admin at evan@preset.io.
**Documentation & Help**
- <a href="https://docs.bito.ai/bito-dev-agents/ai-code-review-agent/getting-started/install-run-using-bito-cloud/create-or-customize-an-agent-instance" target="_blank" rel="noopener noreferrer">Customize agent settings</a>
- <a href="https://docs.bito.ai/bito-dev-agents/ai-code-review-agent/implementing-custom-code-review-rules" target="_blank" rel="noopener noreferrer">Review rules</a>
- <a href="https://docs.bito.ai/bito-dev-agents/ai-code-review-agent" target="_blank" rel="noopener noreferrer">General documentation</a>
- <a href="https://docs.bito.ai/bito-dev-agents/ai-code-review-agent/faqs" target="_blank" rel="noopener noreferrer">FAQ</a>
</details>
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
Re: [PR] fix(assets): Support uploading tags using the assets import endpoint [superset]
Posted by "semohr (via GitHub)" <gi...@apache.org>.
semohr commented on PR #38343:
URL: https://github.com/apache/superset/pull/38343#issuecomment-4084163144
@sfirke Iβve added some unit tests to improve coverage in the Python files. I think I was just confused by the naming of the CI pipeline. I initially assumed `core-package-ts` referred specifically to TypeScript, but itβs probably just shorthand for βtest suite.β
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org