Skip to content

analytics_sdk - #30

Closed
alain-sv wants to merge 3 commits into
developfrom
analytics_sdk
Closed

analytics_sdk#30
alain-sv wants to merge 3 commits into
developfrom
analytics_sdk

Conversation

@alain-sv

Copy link
Copy Markdown
Contributor

Summary Expose analytics resourcesRelated changes- Update analytics changelog

@qodo-code-review

Copy link
Copy Markdown
ⓘ You've reached your Qodo monthly free-tier limit. Reviews pause until next month — upgrade your plan to continue now, or link your paid account if you already have one.

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: c1177ed0a9

ℹ️ About Codex in GitHub

Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".

Comment on lines +98 to +100
"list_dashboards": True,
"get_dashboard": True,
"get_dataset": self.on_get_dataset is not None,

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P1 Badge Set get_dashboard false when no dashboard resolver exists

AnalyticsResource.operations always reports get_dashboard: True, but _make_get_dashboard_handler can only return a dashboard when on_get_dashboard is set or a static dashboard exists. Because check_dashboard_source allows list-only resources (on_list_dashboards without static dashboards), those resources advertise get_dashboard support yet every GET .../dashboards/{id} call returns 404. This breaks clients that trust the registration metadata and then fetch listed dashboard IDs individually.

Useful? React with 👍 / 👎.

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 604ae6cdb7

ℹ️ About Codex in GitHub

Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".

Comment on lines +88 to +89
for dashboard in self.dashboards:
if not dashboard.get("id"):

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P2 Badge Reject duplicate static dashboard IDs

AnalyticsResource validates that each static dashboard has an id, but it does not enforce uniqueness, so two entries can share the same id in dashboards. In that case list_dashboards can return both dashboards while get_static_dashboard always returns only the first match, making later duplicates unreachable and causing inconsistent client behavior when fetching a listed dashboard by id.

Useful? React with 👍 / 👎.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant