echarts icon indicating copy to clipboard operation
echarts copied to clipboard

[Bug] 动态更新x轴数据排序错乱

Open 1163849662 opened this issue 3 years ago • 11 comments

Version

5.33

Link to Minimal Reproduction

No response

Steps to Reproduce

ECharts3 Ajax
<script type="text/javascript">
const myChart = echarts.init(document.getElementById('main'));

  myChart. setOption({
                animation: false,
                tooltip: {
                    trigger: 'axis',
                    confine: true,
                    showContent: false,
                    axisPointer: {
                        lineStyle: {
                            color: 'rgba(0, 0, 0, 0)',
                        }
                    },
                },
                xAxis: {
                    type: 'category',
                    boundaryGap: false,

                },
                yAxis: [
                    {
                        type: 'value',
                        position: 'right',
                        splitNumber: 4,
                        axisTick: {
                            inside: true,
                        },
                        axisLabel: {
                            inside: true,
                            showMinLabel: false,
                        },
                        splitLine: {
                            show: false,
                        }
                    }
                ],
                series: [
                    {
                        data: [],
                        type: 'line',
                        symbol: 'circle',
                        showSymbol: false,
                        symbolSize: 10,
                        itemStyle: {
                            color: 'blue',
                            borderColor: 'yellow',
                            shadowColor: 'rgba(0, 0, 0, 0.5)',
                            shadowBlur: 10
                        },
                        label: {
                            show: true,
                            position: 'right',
                            distance: 10,
                            padding: 10,
                            fontSize: 12,
                            color: '#fff',
                            backgroundColor: 'rgba(0, 0, 0, .6)',
                            formatter: function (params) {
                                return [`价格 :{a|¥${params.data[0]}}`, `总量 :{a|${Math.round(params.data[1])}}`].join('\n')
                            },
                            rich: {
                                a: {
                                    color: '#fff',
                                    fontSize: '12',
                                    fontWeight: 'bold',
                                    lineHeight: '20',
                                },
                            }
                        },
                        lineStyle: {
                            color: '#008c00',
                        },
                        areaStyle: {
                            color: 'green',
                            opacity: .2,
                        },
                    },
                    {
                        //data11[0].reverse()
                        data: [],
                        type: 'line',
                        symbol: 'circle',
                        showSymbol: false,
                        symbolSize: 10,
                        itemStyle: {
                            color: 'blue',
                            borderColor: 'yellow',
                            shadowColor: 'rgba(0, 0, 0, 0.5)',
                            shadowBlur: 10
                        },
                        label: {
                            show: true,
                            position: 'left',
                            distance: 10,
                            padding: 10,
                            fontSize: 12,
                            color: '#fff',
                            backgroundColor: 'rgba(0, 0, 0, .6)',
                            formatter: function (params) {
                                return [`价格 :{a|¥${params.data[0]}}`, `总量 :{a|${Math.round(params.data[1])}}`].join('\n')
                            },
                            rich: {
                                a: {
                                    color: '#fff',
                                    fontSize: '12',
                                    fontWeight: 'bold',
                                    lineHeight: '20',
                                },
                            }
                        },
                        lineStyle: {
                            color: '#ee3523',
                        },
                        areaStyle: {
                            color: 'red',
                            opacity: .2,
                        },
                    },
                ]
            });


//准备好统一的 callback 函数
const update_mychart = function (res) { //res是json格式的response对象

    // 隐藏加载动画
    myChart.hideLoading();

    // 填入数据
    myChart.setOption({
        series: [

                    {
                        //data11[0].reverse()
                        data: res.data[0].reverse(),

                    },
                ]
    });

};

// 首次显示加载动画
myChart.showLoading();


// 建立socket连接,等待服务器“推送”数据,用回调函数更新图表
$(document).ready(function() {
    const namespace = '/test';
    const socket = io.connect(location.protocol + '//' + document.domain + ':' + location.port + namespace);

    socket.on('server_response', function(res) {
        update_mychart(res);
    });

});

</script>

Current Behavior

x轴顺序错 截屏2022-08-26 下午9 39 01 截屏2022-08-26 下午9 38 53 截屏2022-08-26 下午9 38 43

Expected Behavior

如果更正

Environment

- OS:
- Browser:
- Framework:

Any additional comments?

No response

1163849662 avatar Aug 26 '22 13:08 1163849662

nice, thanks for the PR.

A suggestion: perhaps add an example on how to access the installed deployment into NOTES.txt (https://github.com/helm/helm/blob/main/pkg/chartutil/create.go#L438) ?

(I'm not familiar enough with gateway API to know if this is entirely reasonable; but if so, might be helpful to chart consumers/authors?)

Thanks, and great suggestion. I added some infos to the NOTES.txt, but since configuration in the gateway-api are set in Gateway and some in HTTPRoute it is not super easy to give a perfect example for accessing the app since the chart deployment cant look into the Gateway configuration. But I added the infos for accessing it via the most common way (Host and Path based routing) as well on how to get the relevant information from the Gateway

With HTTPRoute enabled and hosts set in the values the output would look like this:

1. Get the application URL by running these commands:
    export APP_HOSTNAME=example.com
    echo "Visit http://$APP_HOSTNAME/headers to use your application"

    NOTE: Your HTTPRoute depends on the listener configuration of your gateway and your HTTPRoute rules.
    These can be set for path, method, header and query parameters.
    You can check the gateway configuration with 'kubectl get --namespace default gateway/gateway -o yaml'

With HTTPRoute enabled and no hosts set in the values the output would look like this:

1. Get the application URL by running these commands:
    export APP_HOSTNAME=$(kubectl get --namespace default gateway/gateway -o jsonpath="{.spec.listeners[0].hostname}")
    echo "Visit http://$APP_HOSTNAME/headers to use your application"

    NOTE: Your HTTPRoute depends on the listener configuration of your gateway and your HTTPRoute rules.
    These can be set for path, method, header and query parameters.
    You can check the gateway configuration with 'kubectl get --namespace default gateway/gateway -o yaml'

Namespaces and Paths are all tempalted form the values/release namespace

hegerdes avatar Mar 28 '24 18:03 hegerdes

Looking good. Thanks for working on this :)

kfox1111 avatar Apr 10 '24 22:04 kfox1111

Thanks for the feedback @sabre1041

  1. Let's align the default hostname among Ingress and HTTPRoute so that there is pariety. Yes, you could potentially install both, but in most cases, you would pick one or the other. So set the default hostname of the HTTPRoute to be chart-example.local

Sure will do!

  1. Support for v1beta1. While the Gateway API is now GA, most who are currently using it is making use of the beta version. Similar to Ingress, there should be a version check to determine which API Group/Version to apply

I thought so too at first but then the question was raised if it is used that much by people, so I removed it. I can revert that without a problem but you also mentioned user experience. I will make the assumption that the people using v1beta1 are technical enough and on the cutting edge side so they will know to change the api version to v1beta1. And if not and they use v1beta1 functions the deployment will just fail. Let the community/maintainers discuss and decide. I have no real preference here.

  1. Overall thoughts: I am hesitant about adding this to the default scaffolded chart. This is mainly from a user experience perspective as we would want the getting started experience to be as smooth as possible. Most users will not have the Gateway API installed and there is a chance that they would face errors when attempting to leverage some of the features that are available in the default scaffolded chart.

I understand that concern. My goal was to make using the GatewayAPI easy because at fist I struggled transforming something usable into helm. To not disturb the user experience the gateway stuff is disabled by default, you have to enable it explicitly and then it should provide a good starting point for your first routes. I see it similar to Ingress and HorizontalPodAutoscaler. A lot of people don't use these and just delete the template but if you need them it is nice to have a good starting point.

hegerdes avatar May 15 '24 08:05 hegerdes

Thanks for the feedback @sabre1041

  1. Let's align the default hostname among Ingress and HTTPRoute so that there is pariety. Yes, you could potentially install both, but in most cases, you would pick one or the other. So set the default hostname of the HTTPRoute to be chart-example.local

Sure will do!

  1. Support for v1beta1. While the Gateway API is now GA, most who are currently using it is making use of the beta version. Similar to Ingress, there should be a version check to determine which API Group/Version to apply

I thought so too at first but then the question was raised if it is used that much by people, so I removed it. I can revert that without a problem but you also mentioned user experience. I will make the assumption that the people using v1beta1 are technical enough and on the cutting edge side so they will know to change the api version to v1beta1. And if not and they use v1beta1 functions the deployment will just fail. Let the community/maintainers discuss and decide. I have no real preference here.

  1. Overall thoughts: I am hesitant about adding this to the default scaffolded chart. This is mainly from a user experience perspective as we would want the getting started experience to be as smooth as possible. Most users will not have the Gateway API installed and there is a chance that they would face errors when attempting to leverage some of the features that are available in the default scaffolded chart.

I understand that concern. My goal was to make using the GatewayAPI easy because at fist I struggled transforming something usable into helm. To not disturb the user experience the gateway stuff is disabled by default, you have to enable it explicitly and then it should provide a good starting point for your first routes. I see it similar to Ingress and HorizontalPodAutoscaler. A lot of people don't use these and just delete the template but if you need them it is nice to have a good starting point.

The biggest concern with including an HTTPRoute with the default set of scaffolded resources during Helm chart creation is that unlike Ingress or HorizontalPodAutoscaler, an HTTPRoute is not an included API within the cluster. So, if they attempt to enable the feature as part of the installation of a chart to the cluster, an error will be returned, not as a result of an error within the contents of the chart, but due to component(s) not being available within the cluster. Its all about the user experience.

sabre1041 avatar May 15 '24 23:05 sabre1041

The Kubernetes folks intend for the gateway api to replace the ingress api though. There is a chicken and the egg problem though. Enough software needs to adopt the ability to use the gateway api before enough clusters deploy it. Clusters are reluctant to deploy it unless software uses it. Software developers are less likely to support it if they don't have a good example of how to do it. Hence this pr's goal to help break the deadlock.

kfox1111 avatar May 16 '24 00:05 kfox1111

Changed the default HTTPRoute hostname in values cording to chart-example.local as suggested by @sabre1041

Rebased from main

hegerdes avatar Jun 08 '24 17:06 hegerdes

Changed the default HTTPRoute hostname in values cording to chart-example.local as suggested by @sabre1041

Rebased from main

Any thoughts about supporting older versions of the Gateway API pre GA?

sabre1041 avatar Jun 09 '24 12:06 sabre1041

Seems like there a no real opinions on that. Also asked in the k8s gateway-api slack channel.

Given that v1.1 is already released I don`t see the real need to support beta versions as I dont think they will be needed once it hits a higher adoption rate.

hegerdes avatar Aug 14 '24 15:08 hegerdes

There was a concern raised (on the Helm weekly developer call), that as the gateway API is not part of the core/standard API. Rather, that is an extension installed via optional CRDs. Helm shouldn't include a "non-standard" (non-core) API as part of a standard chart definition.

(I didn't realize when first seeing this PR, that the API would remain non-core for the foreseeable future, iiuc)

Personally, the Gateway API seems to much closer to being a part of core Kubernetes, than many other CRDs e.g. cert-manager's. But strictly, while it is optionally installed via CRDs. It is never going to be part of a "default" Kubernetes API set.

I specifically asked what Helm maintainers see the function of the helm create scaffolding. And the consensus was the scaffolded chart should be a best practices for a vanilla Kubernetes setup. So in that sense. I agree the Gateway API doesn't meet this standard.

gjenkins8 avatar Aug 21 '24 03:08 gjenkins8

So, a few comments from the Gateway API maintainer POV (that's me!):

  • Gateway API is a set of extension CRDs that are an official part of Kubernetes. They are subject to the same API review and promotion controls that included APIs are in the core Kubernetes binaries. To be honest, Gateway API is the first of many here - this pattern is already adopted by the AdminNetworkPolicy folks, and more will follow. It makes sense for Helm to have a way to handle this "definitely a Kubernetes API but not included by default" issue. (There's a larger issue about ways to ensure that the required version of Gateway API is installed as well, but that's a way larger discussion). As @kfox1111 says, Gateway API and these other APIs are most likely never going to be installed by default though.
  • For Helm charts, I think the way to go is to end up with three settings - hostname, gatewayName, gatewayNamespace, which can be then populated into a HTTPRoute resource and link it up with already-existing infrastructure. Setting one of these should require all three, and the HTTPRoute files should only be included if the values are not empty. (Presumably, this goes for helm create as well).

I'm happy to come to a Helm community call or something at some point to discuss this in a higher-bandwidth way, or to meet up at Kubecon NA and talk there too.

There are two Helm-related problems that have been on my mind for some time:

  • How to help chart authors include HTTPRoutes more easily (that's this one)
  • How to help chart authors say "Gateway API being installed is a dependency" and "Gateway API must be at least this version", both of which are surprisingly hard problems that will take a while to work out. But having more communication between Gateway API folks and Helm folks here will help a lot, I think.

youngnick avatar Aug 22 '24 05:08 youngnick

Apologies for the belated reply. I agree with the above. And I would like to work out how to move forward.

Overall, I'm not exactly sure what the answers are. I think the question is less "how does Helm handle HTTPRoute and gateway API?", and more: "How does Helm (and other tooling) handle 'out-of-tree' types?"

I do think worthy to try and sync at KubeCon. I will be there, as well as other Helm maintainers. And if possible, jumping on the Helm developer call (9:30am Pacific, Thursdays; see #helm-dev on Kubernetes slack for zoom link and password) to try and prompt some attention might help too.

gjenkins8 avatar Oct 01 '24 15:10 gjenkins8

Thanks for approving and your feedback @gjenkins8. I will update the comment tomorrow and will also rebase.

Just out of interest: Did the topic of any none core kubernetes-apis come up at kubecon? I was not able to attend, but am quite interested if there where opinions exchanged. Regarding the role of helm and gateway-api

hegerdes avatar Nov 19 '24 18:11 hegerdes

Anything left to do on this?

kfox1111 avatar Dec 19 '24 20:12 kfox1111

Whats left to get this in?

kfox1111 avatar Feb 06 '25 22:02 kfox1111

Implemented the requested changes to values.yaml template

hegerdes avatar Feb 10 '25 09:02 hegerdes

@hegerdes -- could you fix the conflicts please. I didn't see Andy's prior approval (otherwise I would have merged). Once conflicts are fixed, we should be able to do a quick round of re-approvals and get this merged.

gjenkins8 avatar Mar 10 '25 23:03 gjenkins8

@gjenkins8 No problem!

No code changes required, git handled the modified repo structure just fine in a simple rebase

hegerdes avatar Mar 11 '25 16:03 hegerdes

Thanks for the patience @hegerdes

Since main is now targeting Helm v4, I've created a backport PR to dev-v3 branch: https://github.com/helm/helm/pull/30658

gjenkins8 avatar Mar 12 '25 17:03 gjenkins8