Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

chore: limit graphColorMapping to CLOUD #6641

Draft
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

wdoconnell
Copy link
Contributor

@wdoconnell wdoconnell commented Mar 3, 2023

Part of #6307

@jeffreyssmith2nd traced this bug to this PR adding color persistence features (graphColorMapping) to UI visualizations. It was part of this epic.

#5233 removed feature flag protection on that feature, resulting in it becoming part of the UI codebase for OSS and Cloud. It's causing serious visualization rendering issues in OSS (but not CLOUD, seemingly). To unblock the OSS team, which has the UI pinned to an older version to resolve this issue, we're going to proceed without the graphColorMapping feature in OSS, at least for now, so that OSS doesn't need to stay pinned to an outdated UI.

Checklist

Authors and Reviewer(s), please verify the following:

  • A PR description, regardless of the triviality of this change, that communicates the value of this PR
  • Well-formatted conventional commit messages that provide context into the change
  • Documentation updated or issue created (provide link to issue/PR)
  • Signed CLA (if not already signed)
  • Feature flagged, if applicable - NO, but only removes color mapping from prospective OSS UI.

@jeffreyssmith2nd
Copy link
Contributor

Hey @wdoconnell, is there anything needed from my end to help get this PR merged? I'd love to get it into the 2.7 release of InfluxDB (date TBD) if possible.

@wdoconnell
Copy link
Contributor Author

Hi @jeffreyssmith2nd - this needs some additional testing, which I expect to be able to get to this week (likely Thursday/Friday). If no issues arise we should be able to merge it then.

@jeffreyssmith2nd
Copy link
Contributor

Great, thank you @wdoconnell!

@wdoconnell
Copy link
Contributor Author

@jeffreyssmith2nd Due to some reprioritization of golang work in granite work, testing of this is likely moved to early next week.

If I'm still able to get through testing of it this week, I'll let you know. 🙏

@jeffreyssmith2nd
Copy link
Contributor

@jeffreyssmith2nd Due to some reprioritization of golang work in granite work, testing of this is likely moved to early next week.

If I'm still able to get through testing of it this week, I'll let you know. 🙏

No worries, I appreciate you taking a look at it!

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.

2 participants