Re: [PR] feat: custom refresh frequency [superset]
Abhishek-kumar-samsung commented on PR #24449: URL: https://github.com/apache/superset/pull/24449#issuecomment-2079226563 > Thanks @Abhishek-kumar-samsung - I/we appreciate you sticking with this and getting it through! Thankyou @rusackas -- 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-unsubscr...@superset.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org - To unsubscribe, e-mail: notifications-unsubscr...@superset.apache.org For additional commands, e-mail: notifications-h...@superset.apache.org
Re: [PR] feat: custom refresh frequency [superset]
rusackas commented on PR #24449: URL: https://github.com/apache/superset/pull/24449#issuecomment-2075111993 Thanks @Abhishek-kumar-samsung - I/we appreciate you sticking with this and getting 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-unsubscr...@superset.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org - To unsubscribe, e-mail: notifications-unsubscr...@superset.apache.org For additional commands, e-mail: notifications-h...@superset.apache.org
Re: [PR] feat: custom refresh frequency [superset]
rusackas merged PR #24449: URL: https://github.com/apache/superset/pull/24449 -- 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-unsubscr...@superset.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org - To unsubscribe, e-mail: notifications-unsubscr...@superset.apache.org For additional commands, e-mail: notifications-h...@superset.apache.org
Re: [PR] feat: custom refresh frequency [superset]
Abhishek-kumar-samsung commented on PR #24449: URL: https://github.com/apache/superset/pull/24449#issuecomment-2074005750 > @Abhishek-kumar-samsung it seems that CI is stuck. Please pull latest master and push back to kick CI on a fresh master state. Thank you Hi @geido i synced my fork to latest main branch, i don't know why the pull_request check is failing. From code side it did not gave any check failure. Last time when it ran, the same code passed all checks. -- 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-unsubscr...@superset.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org - To unsubscribe, e-mail: notifications-unsubscr...@superset.apache.org For additional commands, e-mail: notifications-h...@superset.apache.org
Re: [PR] feat: custom refresh frequency [superset]
geido commented on PR #24449: URL: https://github.com/apache/superset/pull/24449#issuecomment-2072292405 @Abhishek-kumar-samsung it seems that CI is stuck. Please pull latest master and push back to kick CI on a fresh master state. Thank you -- 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-unsubscr...@superset.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org - To unsubscribe, e-mail: notifications-unsubscr...@superset.apache.org For additional commands, e-mail: notifications-h...@superset.apache.org
Re: [PR] feat: custom refresh frequency [superset]
rusackas commented on PR #24449: URL: https://github.com/apache/superset/pull/24449#issuecomment-2047857845 @geido can you check your requested changes to (potentially) unblock the merge when you have a chance, please? -- 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-unsubscr...@superset.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org - To unsubscribe, e-mail: notifications-unsubscr...@superset.apache.org For additional commands, e-mail: notifications-h...@superset.apache.org
Re: [PR] feat: custom refresh frequency [superset]
Abhishek-kumar-samsung commented on PR #24449: URL: https://github.com/apache/superset/pull/24449#issuecomment-2046586892 I accidently closed the PR and I reopened it again. -- 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-unsubscr...@superset.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org - To unsubscribe, e-mail: notifications-unsubscr...@superset.apache.org For additional commands, e-mail: notifications-h...@superset.apache.org
Re: [PR] feat: custom refresh frequency [superset]
codecov-commenter commented on PR #24449: URL: https://github.com/apache/superset/pull/24449#issuecomment-2046586087 ## [Codecov](https://app.codecov.io/gh/apache/superset/pull/24449?dropdown=coverage=pr=h1_medium=referral_source=github_content=comment_campaign=pr+comments_term=apache) Report Attention: Patch coverage is `40.0%` with `24 lines` in your changes are missing coverage. Please review. > Project coverage is 67.41%. Comparing base [(`9ced255`)](https://app.codecov.io/gh/apache/superset/commit/9ced2552dbeeaf60217b385d4c40cbaf4372c787?dropdown=coverage=desc_medium=referral_source=github_content=comment_campaign=pr+comments_term=apache) to head [(`4bd1582`)](https://app.codecov.io/gh/apache/superset/pull/24449?dropdown=coverage=pr=desc_medium=referral_source=github_content=comment_campaign=pr+comments_term=apache). > Report is 226 commits behind head on master. | [Files](https://app.codecov.io/gh/apache/superset/pull/24449?dropdown=coverage=pr=tree_medium=referral_source=github_content=comment_campaign=pr+comments_term=apache) | Patch % | Lines | |---|---|---| | [.../src/dashboard/components/RefreshIntervalModal.tsx](https://app.codecov.io/gh/apache/superset/pull/24449?src=pr=tree=superset-frontend%2Fsrc%2Fdashboard%2Fcomponents%2FRefreshIntervalModal.tsx_medium=referral_source=github_content=comment_campaign=pr+comments_term=apache#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL2Rhc2hib2FyZC9jb21wb25lbnRzL1JlZnJlc2hJbnRlcnZhbE1vZGFsLnRzeA==) | 40.00% | [23 Missing and 1 partial :warning: ](https://app.codecov.io/gh/apache/superset/pull/24449?src=pr=tree_medium=referral_source=github_content=comment_campaign=pr+comments_term=apache) | Additional details and impacted files ```diff @@Coverage Diff @@ ## master #24449 +/- ## == + Coverage 67.34% 67.41% +0.07% == Files1909 1910 +1 Lines 7462374741 +118 Branches 8324 8356 +32 == + Hits5025650388 +132 + Misses 2231422300 -14 Partials 2053 2053 ``` | [Flag](https://app.codecov.io/gh/apache/superset/pull/24449/flags?src=pr=flags_medium=referral_source=github_content=comment_campaign=pr+comments_term=apache) | Coverage Δ | | |---|---|---| | [javascript](https://app.codecov.io/gh/apache/superset/pull/24449/flags?src=pr=flag_medium=referral_source=github_content=comment_campaign=pr+comments_term=apache) | `57.37% <40.00%> (+0.16%)` | :arrow_up: | Flags with carried forward coverage won't be shown. [Click here](https://docs.codecov.io/docs/carryforward-flags?utm_medium=referral_source=github_content=comment_campaign=pr+comments_term=apache#carryforward-flags-in-the-pull-request-comment) to find out more. [:umbrella: View full report in Codecov by Sentry](https://app.codecov.io/gh/apache/superset/pull/24449?dropdown=coverage=pr=continue_medium=referral_source=github_content=comment_campaign=pr+comments_term=apache). :loudspeaker: Have feedback on the report? [Share it here](https://about.codecov.io/codecov-pr-comment-feedback/?utm_medium=referral_source=github_content=comment_campaign=pr+comments_term=apache). -- 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-unsubscr...@superset.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org - To unsubscribe, e-mail: notifications-unsubscr...@superset.apache.org For additional commands, e-mail: notifications-h...@superset.apache.org
[PR] feat: custom refresh frequency [superset]
Abhishek-kumar-samsung opened a new pull request, #24449: URL: https://github.com/apache/superset/pull/24449 Earlier at dashboard level we had very limited options in 'Set auto-refresh interval' as shown in image below   But as per our organisation level we wanted the frequency to be something else that was not in dropdown, so we implemented that. As part of this PR, we can add custom frequency as required by user, user can select any frequency in hour, minute and seconds and set it as auto refresh interval frequency (in seconds).    -- 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-unsubscr...@superset.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org - To unsubscribe, e-mail: notifications-unsubscr...@superset.apache.org For additional commands, e-mail: notifications-h...@superset.apache.org
Re: [PR] feat: custom refresh frequency [superset]
github-actions[bot] commented on PR #24449: URL: https://github.com/apache/superset/pull/24449#issuecomment-2046584091 Ephemeral environment shutdown and build artifacts deleted. -- 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-unsubscr...@superset.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org - To unsubscribe, e-mail: notifications-unsubscr...@superset.apache.org For additional commands, e-mail: notifications-h...@superset.apache.org
Re: [PR] feat: custom refresh frequency [superset]
Abhishek-kumar-samsung commented on PR #24449: URL: https://github.com/apache/superset/pull/24449#issuecomment-2046583583 > > Hi all - please direct me to an issue link if there's a separate place for questions. We recently adopted Superset and I'm looking into setting up refresh schedules for our dashboards. This PR comes close to introducing the feature I think we need, but really something with the flexibility like cron schedule expressions would be ideal. I haven't been able to understand when the refresh interval triggers (i.e. how it's determined when it'll happen) or to specify a time it should kick off. > > Effectively I just want to specify when my refresh interval should start and (based off the time window I choose) when it should stop. Can any help me understand this, please? > > Hi there, cron refresh would be an ideal solution to tell which dashboard should refresh all their pages at which time, based on the refresh schedule of underneath dataset. Hi @rusackas I wonder is it ideal similar to the cache warm up feature, could you please take a look at this idea or fix the cache warm up feature, so that users will always be able to view the lastest update data without clicking into 'Force refresh' on charts or 'Refresh dashboard' on top. Thank you -- 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-unsubscr...@superset.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org - To unsubscribe, e-mail: notifications-unsubscr...@superset.apache.org For additional commands, e-mail: notifications-h...@superset.apache.org
Re: [PR] feat: custom refresh frequency [superset]
Abhishek-kumar-samsung closed pull request #24449: feat: custom refresh frequency URL: https://github.com/apache/superset/pull/24449 -- 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-unsubscr...@superset.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org - To unsubscribe, e-mail: notifications-unsubscr...@superset.apache.org For additional commands, e-mail: notifications-h...@superset.apache.org
Re: [PR] feat: custom refresh frequency [superset]
rusackas commented on PR #24449: URL: https://github.com/apache/superset/pull/24449#issuecomment-2046038294 After some quick sanity tests on the ephemeral, this is looking pretty good! -- 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-unsubscr...@superset.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org - To unsubscribe, e-mail: notifications-unsubscr...@superset.apache.org For additional commands, e-mail: notifications-h...@superset.apache.org
Re: [PR] feat: custom refresh frequency [superset]
LeoDiep commented on PR #24449: URL: https://github.com/apache/superset/pull/24449#issuecomment-2044155114 > Hi all - please direct me to an issue link if there's a separate place for questions. We recently adopted Superset and I'm looking into setting up refresh schedules for our dashboards. This PR comes close to introducing the feature I think we need, but really something with the flexibility like cron schedule expressions would be ideal. I haven't been able to understand when the refresh interval triggers (i.e. how it's determined when it'll happen) or to specify a time it should kick off. > > Effectively I just want to specify when my refresh interval should start and (based off the time window I choose) when it should stop. Can any help me understand this, please? Hi there, cron refresh would be an ideal solution to tell which dashboard should refresh all their pages at which time, based on the refresh schedule of underneath dataset. Hi @rusackas I wonder is it ideal similar to the cache warm up feature, could you please take a look at this idea or fix the cache warm up feature, so that users will always be able to view the lastest update data without clicking into 'Force refresh' on charts or 'Refresh dashboard' on top. Thank you -- 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-unsubscr...@superset.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org - To unsubscribe, e-mail: notifications-unsubscr...@superset.apache.org For additional commands, e-mail: notifications-h...@superset.apache.org
Re: [PR] feat: custom refresh frequency [superset]
github-actions[bot] commented on PR #24449: URL: https://github.com/apache/superset/pull/24449#issuecomment-2033482292 @rusackas Ephemeral environment spinning up at http://54.245.47.10:8080. Credentials are `admin`/`admin`. Please allow several minutes for bootstrapping and startup. -- 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-unsubscr...@superset.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org - To unsubscribe, e-mail: notifications-unsubscr...@superset.apache.org For additional commands, e-mail: notifications-h...@superset.apache.org
Re: [PR] feat: custom refresh frequency [superset]
rusackas commented on PR #24449: URL: https://github.com/apache/superset/pull/24449#issuecomment-2033474686 /testenv up -- 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-unsubscr...@superset.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org - To unsubscribe, e-mail: notifications-unsubscr...@superset.apache.org For additional commands, e-mail: notifications-h...@superset.apache.org
Re: [PR] feat: custom refresh frequency [superset]
Abhishek-kumar-samsung commented on code in PR #24449:
URL: https://github.com/apache/superset/pull/24449#discussion_r1535001895
##
superset-frontend/src/dashboard/components/RefreshIntervalModal.tsx:
##
@@ -140,16 +256,52 @@ class RefreshIntervalModal extends React.PureComponent<
}
modalFooter={
<>
+
+ {t('Cancel')}
+
{
Review Comment:
Done
##
superset-frontend/src/dashboard/components/RefreshIntervalModal.tsx:
##
@@ -111,17 +165,79 @@ class RefreshIntervalModal extends React.PureComponent<
modalTitle={t('Refresh interval')}
modalBody={
-{t('Refresh frequency')}
- ({
-value: option[0],
-label: t(option[1]),
- }))}
- value={refreshFrequency}
- onChange={this.handleFrequencyChange}
- sortComparator={propertyComparator('value')}
-/>
+
+
+{t('Refresh frequency')}
+
+
+
+{custom_block && (
+
Re: [PR] feat: custom refresh frequency [superset]
Abhishek-kumar-samsung commented on PR #24449: URL: https://github.com/apache/superset/pull/24449#issuecomment-2014282484 @geido @rusackas i resolved whatever issues pointed out, can you pls 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-unsubscr...@superset.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org - To unsubscribe, e-mail: notifications-unsubscr...@superset.apache.org For additional commands, e-mail: notifications-h...@superset.apache.org
Re: [PR] feat: custom refresh frequency [superset]
Abhishek-kumar-samsung commented on PR #24449: URL: https://github.com/apache/superset/pull/24449#issuecomment-2011341422 > I agree we don't want to grow the scope of this PR. We should merge this first, and then deal with the cron idea (which is a nice idea) as a subsequent PR. > > For the cron feature, it seems like a reasonable plan to seek "lazy consensus" on the dev@ list. Then if that starts an argument, we can upgrade the conversation to a SIP and vote it. Yes sure, i will quickly do the two changes that you had suggested and once merged, we will raise SIP for cron. Then we can work on it, @coetzeevs you can also work on that if you had implemented something in 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-unsubscr...@superset.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org - To unsubscribe, e-mail: notifications-unsubscr...@superset.apache.org For additional commands, e-mail: notifications-h...@superset.apache.org
Re: [PR] feat: custom refresh frequency [superset]
rusackas commented on PR #24449: URL: https://github.com/apache/superset/pull/24449#issuecomment-2011316130 I agree we don't want to grow the scope of this PR. We should merge this first, and then deal with the cron idea (which is a nice idea) as a subsequent PR. For the cron feature, it seems like a reasonable plan to seek "lazy consensus" on the dev@ list. Then if that starts an argument, we can upgrade the conversation to a SIP and vote 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-unsubscr...@superset.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org - To unsubscribe, e-mail: notifications-unsubscr...@superset.apache.org For additional commands, e-mail: notifications-h...@superset.apache.org
Re: [PR] feat: custom refresh frequency [superset]
Abhishek-kumar-samsung commented on PR #24449: URL: https://github.com/apache/superset/pull/24449#issuecomment-2008650960 > Hi all - please direct me to an issue link if there's a separate place for questions. We recently adopted Superset and I'm looking into setting up refresh schedules for our dashboards. This PR comes close to introducing the feature I think we need, but really something with the flexibility like cron schedule expressions would be ideal. I haven't been able to understand when the refresh interval triggers (i.e. how it's determined when it'll happen) or to specify a time it should kick off. > > Effectively I just want to specify when my refresh interval should start and (based off the time window I choose) when it should stop. Can any help me understand this, please? Currently in this PR, i was implementing refresh intervals based on relative timing. But crons idea would be nice. If we need to include crons then this PR will extend to some long time, maybe endlessly. So maybe we can start an SIP with multiple stages of PR, First stage maybe this one i.e relative time, and Second stage can be crons. What say? @rusackas -- 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-unsubscr...@superset.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org - To unsubscribe, e-mail: notifications-unsubscr...@superset.apache.org For additional commands, e-mail: notifications-h...@superset.apache.org
Re: [PR] feat: custom refresh frequency [superset]
coetzeevs commented on PR #24449: URL: https://github.com/apache/superset/pull/24449#issuecomment-2008054712 Hi all - please direct me to an issue link if there's a separate place for questions. We recently adopted Superset and I'm looking into setting up refresh schedules for our dashboards. This PR comes close to introducing the feature I think we need, but really something with the flexibility like cron schedule expressions would be ideal. I haven't been able to understand when the refresh interval triggers (i.e. how it's determined when it'll happen) or to specify a time it should kick off. Effectively I just want to specify when my refresh interval should start and (based off the time window I choose) when it should stop. Can any help me understand this, please? -- 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-unsubscr...@superset.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org - To unsubscribe, e-mail: notifications-unsubscr...@superset.apache.org For additional commands, e-mail: notifications-h...@superset.apache.org
Re: [PR] feat: custom refresh frequency [superset]
rusackas commented on code in PR #24449:
URL: https://github.com/apache/superset/pull/24449#discussion_r1521969620
##
superset-frontend/src/dashboard/components/RefreshIntervalModal.tsx:
##
@@ -111,17 +165,79 @@ class RefreshIntervalModal extends React.PureComponent<
modalTitle={t('Refresh interval')}
modalBody={
-{t('Refresh frequency')}
- ({
-value: option[0],
-label: t(option[1]),
- }))}
- value={refreshFrequency}
- onChange={this.handleFrequencyChange}
- sortComparator={propertyComparator('value')}
-/>
+
+
+{t('Refresh frequency')}
+
+
+
+{custom_block && (
+ https://github.com/apache/superset/wiki/Emotion-Styling-Guidelines-and-Best-Practices).
--
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-unsubscr...@superset.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
-
To unsubscribe, e-mail: notifications-unsubscr...@superset.apache.org
For additional commands, e-mail: notifications-h...@superset.apache.org
Re: [PR] feat: custom refresh frequency [superset]
rusackas commented on code in PR #24449:
URL: https://github.com/apache/superset/pull/24449#discussion_r1521967561
##
superset-frontend/src/dashboard/components/RefreshIntervalModal.tsx:
##
@@ -140,16 +256,52 @@ class RefreshIntervalModal extends React.PureComponent<
}
modalFooter={
<>
+
+ {t('Cancel')}
+
{
Review Comment:
It would be better if handlers called an external function rather than
defining the logic inline.
--
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-unsubscr...@superset.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
-
To unsubscribe, e-mail: notifications-unsubscr...@superset.apache.org
For additional commands, e-mail: notifications-h...@superset.apache.org
Re: [PR] feat: custom refresh frequency [superset]
github-actions[bot] commented on PR #24449: URL: https://github.com/apache/superset/pull/24449#issuecomment-1989186475 @geido Ephemeral environment spinning up at http://34.220.148.99:8080. Credentials are `admin`/`admin`. Please allow several minutes for bootstrapping and startup. -- 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-unsubscr...@superset.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org - To unsubscribe, e-mail: notifications-unsubscr...@superset.apache.org For additional commands, e-mail: notifications-h...@superset.apache.org
Re: [PR] feat: custom refresh frequency [superset]
geido commented on PR #24449: URL: https://github.com/apache/superset/pull/24449#issuecomment-1989168669 /testenv up -- 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-unsubscr...@superset.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org - To unsubscribe, e-mail: notifications-unsubscr...@superset.apache.org For additional commands, e-mail: notifications-h...@superset.apache.org
Re: [PR] feat: custom refresh frequency [superset]
Abhishek-kumar-samsung commented on PR #24449: URL: https://github.com/apache/superset/pull/24449#issuecomment-1985657686 @geido can you pls review again :) I have made modifications as asked by you. -- 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-unsubscr...@superset.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org - To unsubscribe, e-mail: notifications-unsubscr...@superset.apache.org For additional commands, e-mail: notifications-h...@superset.apache.org
Re: [PR] feat: custom refresh frequency [superset]
geido commented on code in PR #24449:
URL: https://github.com/apache/superset/pull/24449#discussion_r1507886383
##
superset-frontend/src/dashboard/components/RefreshIntervalModal.tsx:
##
@@ -91,6 +100,49 @@ class RefreshIntervalModal extends React.PureComponent<
this.setState({
refreshFrequency: value || refreshIntervalOptions[0][0],
});
+
+this.setState({
+ custom_block: value === -1,
+});
+
+if (value === -1) {
+ this.setState({
+custom_hour: 0,
+custom_min: 0,
+custom_sec: 0,
+ });
+}
+ }
+
+ onSaveValue(value: number) {
+this.props.onChange(value, this.props.editMode);
+this.modalRef?.current?.close();
+this.props.addSuccessToast(t('Refresh interval saved'));
+ }
+
+ createIntervalOptions(refreshIntervalOptions: [number, string][]) {
+const refresh_options = [];
+if (!refreshIntervalOptions.length) {
+ refresh_options.push({ value: -1, label: 'Custom interval' });
+ return refresh_options;
+}
+
+refresh_options.push({ value: -1, label: 'Custom interval' });
Review Comment:
What I mean is that you are duplicating the code by using the condition
`!refreshIntervalOptions.length` and then pushing the custom interval option,
but you also do the same when `refreshIntervalOptions.length` is larger than 0.
So just do:
```
refresh_options.push({ value: -1, label: 'Custom interval' });
if (refreshIntervalOptions.length) {
refresh_options.push(
...refreshIntervalOptions.map(option => ({
value: option[0],
label: t(option[1]),
})),
);
}
return refresh_options;
```
--
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-unsubscr...@superset.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
-
To unsubscribe, e-mail: notifications-unsubscr...@superset.apache.org
For additional commands, e-mail: notifications-h...@superset.apache.org
Re: [PR] feat: custom refresh frequency [superset]
geido commented on code in PR #24449:
URL: https://github.com/apache/superset/pull/24449#discussion_r1507880813
##
superset-frontend/src/dashboard/components/RefreshIntervalModal.tsx:
##
@@ -111,17 +169,78 @@ class RefreshIntervalModal extends React.PureComponent<
modalTitle={t('Refresh interval')}
modalBody={
-{t('Refresh frequency')}
- ({
-value: option[0],
-label: t(option[1]),
- }))}
- value={refreshFrequency}
- onChange={this.handleFrequencyChange}
- sortComparator={propertyComparator('value')}
-/>
+
+
+{t('Refresh frequency')}
+
+
+
+
Re: [PR] feat: custom refresh frequency [superset]
Abhishek-kumar-samsung commented on code in PR #24449:
URL: https://github.com/apache/superset/pull/24449#discussion_r1507778658
##
superset-frontend/src/dashboard/components/RefreshIntervalModal.tsx:
##
@@ -111,17 +169,78 @@ class RefreshIntervalModal extends React.PureComponent<
modalTitle={t('Refresh interval')}
modalBody={
-{t('Refresh frequency')}
- ({
-value: option[0],
-label: t(option[1]),
- }))}
- value={refreshFrequency}
- onChange={this.handleFrequencyChange}
- sortComparator={propertyComparator('value')}
-/>
+
+
+{t('Refresh frequency')}
+
+
+
+
Re: [PR] feat: custom refresh frequency [superset]
Abhishek-kumar-samsung commented on code in PR #24449:
URL: https://github.com/apache/superset/pull/24449#discussion_r1507774260
##
superset-frontend/src/dashboard/components/RefreshIntervalModal.tsx:
##
@@ -91,6 +100,49 @@ class RefreshIntervalModal extends React.PureComponent<
this.setState({
refreshFrequency: value || refreshIntervalOptions[0][0],
});
+
+this.setState({
+ custom_block: value === -1,
+});
+
+if (value === -1) {
+ this.setState({
+custom_hour: 0,
+custom_min: 0,
+custom_sec: 0,
+ });
+}
+ }
+
+ onSaveValue(value: number) {
+this.props.onChange(value, this.props.editMode);
+this.modalRef?.current?.close();
+this.props.addSuccessToast(t('Refresh interval saved'));
+ }
+
+ createIntervalOptions(refreshIntervalOptions: [number, string][]) {
+const refresh_options = [];
+if (!refreshIntervalOptions.length) {
+ refresh_options.push({ value: -1, label: 'Custom interval' });
+ return refresh_options;
+}
+
+refresh_options.push({ value: -1, label: 'Custom interval' });
Review Comment:
Ok i will localize it,
no i am not adding twice because from if condition i am returning.
--
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-unsubscr...@superset.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
-
To unsubscribe, e-mail: notifications-unsubscr...@superset.apache.org
For additional commands, e-mail: notifications-h...@superset.apache.org
Re: [PR] feat: custom refresh frequency [superset]
geido commented on code in PR #24449:
URL: https://github.com/apache/superset/pull/24449#discussion_r1502947003
##
superset-frontend/src/dashboard/components/RefreshIntervalModal.tsx:
##
@@ -91,6 +100,49 @@ class RefreshIntervalModal extends React.PureComponent<
this.setState({
refreshFrequency: value || refreshIntervalOptions[0][0],
});
+
+this.setState({
+ custom_block: value === -1,
+});
+
+if (value === -1) {
+ this.setState({
+custom_hour: 0,
+custom_min: 0,
+custom_sec: 0,
+ });
+}
+ }
+
+ onSaveValue(value: number) {
+this.props.onChange(value, this.props.editMode);
+this.modalRef?.current?.close();
+this.props.addSuccessToast(t('Refresh interval saved'));
+ }
+
+ createIntervalOptions(refreshIntervalOptions: [number, string][]) {
+const refresh_options = [];
+if (!refreshIntervalOptions.length) {
+ refresh_options.push({ value: -1, label: 'Custom interval' });
+ return refresh_options;
+}
+
+refresh_options.push({ value: -1, label: 'Custom interval' });
Review Comment:
The label needs to be localized. Also, aren't you adding this two times as
you are adding it when the above condition triggers too?
##
superset-frontend/src/dashboard/components/RefreshIntervalModal.tsx:
##
@@ -111,17 +169,78 @@ class RefreshIntervalModal extends React.PureComponent<
modalTitle={t('Refresh interval')}
modalBody={
-{t('Refresh frequency')}
- ({
-value: option[0],
-label: t(option[1]),
- }))}
- value={refreshFrequency}
- onChange={this.handleFrequencyChange}
- sortComparator={propertyComparator('value')}
-/>
+
+
+{t('Refresh frequency')}
+
+
+
+
Re: [PR] feat: custom refresh frequency [superset]
geido commented on code in PR #24449:
URL: https://github.com/apache/superset/pull/24449#discussion_r1502945531
##
superset-frontend/src/dashboard/components/RefreshIntervalModal.tsx:
##
@@ -91,6 +100,49 @@ class RefreshIntervalModal extends React.PureComponent<
this.setState({
refreshFrequency: value || refreshIntervalOptions[0][0],
});
+
+this.setState({
+ custom_block: value === -1,
+});
+
+if (value === -1) {
+ this.setState({
+custom_hour: 0,
+custom_min: 0,
+custom_sec: 0,
+ });
+}
+ }
+
+ onSaveValue(value: number) {
+this.props.onChange(value, this.props.editMode);
+this.modalRef?.current?.close();
+this.props.addSuccessToast(t('Refresh interval saved'));
+ }
+
+ createIntervalOptions(refreshIntervalOptions: [number, string][]) {
+const refresh_options = [];
+if (!refreshIntervalOptions.length) {
+ refresh_options.push({ value: -1, label: 'Custom interval' });
Review Comment:
I think the label here need to be localized
--
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-unsubscr...@superset.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
-
To unsubscribe, e-mail: notifications-unsubscr...@superset.apache.org
For additional commands, e-mail: notifications-h...@superset.apache.org
Re: [PR] feat: custom refresh frequency [superset]
github-actions[bot] commented on PR #24449: URL: https://github.com/apache/superset/pull/24449#issuecomment-1961835267 @geido Ephemeral environment spinning up at http://54.149.118.241:8080. Credentials are `admin`/`admin`. Please allow several minutes for bootstrapping and startup. -- 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-unsubscr...@superset.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org - To unsubscribe, e-mail: notifications-unsubscr...@superset.apache.org For additional commands, e-mail: notifications-h...@superset.apache.org
Re: [PR] feat: custom refresh frequency [superset]
geido commented on PR #24449: URL: https://github.com/apache/superset/pull/24449#issuecomment-1961820984 /testenv up -- 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-unsubscr...@superset.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org - To unsubscribe, e-mail: notifications-unsubscr...@superset.apache.org For additional commands, e-mail: notifications-h...@superset.apache.org
Re: [PR] feat: custom refresh frequency [superset]
Abhishek-kumar-samsung commented on PR #24449: URL: https://github.com/apache/superset/pull/24449#issuecomment-1961683193 > Thanks for the contribution! There are some important changes required from a first-pass review. Let me know when you are ready for another round and I will be happy to have another look @geido Can you pls have another look, i resolved all the things that you had mentioned. -- 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-unsubscr...@superset.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org - To unsubscribe, e-mail: notifications-unsubscr...@superset.apache.org For additional commands, e-mail: notifications-h...@superset.apache.org
Re: [PR] feat: custom refresh frequency [superset]
maanwater commented on PR #24449: URL: https://github.com/apache/superset/pull/24449#issuecomment-1961319984 Do you think this will make it to the 4.0.0 release? -- 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-unsubscr...@superset.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org - To unsubscribe, e-mail: notifications-unsubscr...@superset.apache.org For additional commands, e-mail: notifications-h...@superset.apache.org
Re: [PR] feat: custom refresh frequency [superset]
geido commented on code in PR #24449:
URL: https://github.com/apache/superset/pull/24449#discussion_r1495575011
##
superset-frontend/src/dashboard/components/RefreshIntervalModal.tsx:
##
@@ -91,6 +99,51 @@ class RefreshIntervalModal extends React.PureComponent<
this.setState({
refreshFrequency: value || refreshIntervalOptions[0][0],
});
+
+if (value === -1) {
+ this.setState({
+custom_block: true,
+ });
+} else {
+ this.setState({
+custom_block: false,
+ });
+}
+ }
+
+ onSaveValue(value: number) {
+this.props.onChange(value, this.props.editMode);
+this.modalRef?.current?.close();
+this.props.addSuccessToast(t('Refresh interval saved'));
+ }
+
+ createIntervalOptions(refreshIntervalOptions: [number, string][]) {
+const refresh_options = [];
+if (refreshIntervalOptions.length === 0) {
+ refresh_options.push({ value: -1, label: 'Custom interval' });
+ return refresh_options;
+}
+refresh_options.push({
+ value: refreshIntervalOptions[0][0],
+ label: t(refreshIntervalOptions[0][1]),
+});
+refresh_options.push({ value: -1, label: 'Custom interval' });
+for (let i = 1; i < refreshIntervalOptions.length; i += 1)
+ refresh_options.push({
+value: refreshIntervalOptions[i][0],
+label: t(refreshIntervalOptions[i][1]),
+ });
+return refresh_options;
+ }
Review Comment:
Ok then please consider using `map` rather than the for loop to make the
code more readable and concise. Thank you
--
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-unsubscr...@superset.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
-
To unsubscribe, e-mail: notifications-unsubscr...@superset.apache.org
For additional commands, e-mail: notifications-h...@superset.apache.org
Re: [PR] feat: custom refresh frequency [superset]
Abhishek-kumar-samsung commented on code in PR #24449:
URL: https://github.com/apache/superset/pull/24449#discussion_r1495228835
##
superset-frontend/src/dashboard/components/RefreshIntervalModal.tsx:
##
@@ -91,6 +99,51 @@ class RefreshIntervalModal extends React.PureComponent<
this.setState({
refreshFrequency: value || refreshIntervalOptions[0][0],
});
+
+if (value === -1) {
+ this.setState({
+custom_block: true,
+ });
+} else {
+ this.setState({
+custom_block: false,
+ });
+}
+ }
+
+ onSaveValue(value: number) {
+this.props.onChange(value, this.props.editMode);
+this.modalRef?.current?.close();
+this.props.addSuccessToast(t('Refresh interval saved'));
+ }
+
+ createIntervalOptions(refreshIntervalOptions: [number, string][]) {
+const refresh_options = [];
+if (refreshIntervalOptions.length === 0) {
+ refresh_options.push({ value: -1, label: 'Custom interval' });
+ return refresh_options;
+}
+refresh_options.push({
+ value: refreshIntervalOptions[0][0],
+ label: t(refreshIntervalOptions[0][1]),
+});
+refresh_options.push({ value: -1, label: 'Custom interval' });
+for (let i = 1; i < refreshIntervalOptions.length; i += 1)
+ refresh_options.push({
+value: refreshIntervalOptions[i][0],
+label: t(refreshIntervalOptions[i][1]),
+ });
+return refresh_options;
+ }
Review Comment:
Yes i kept custom options in the array options that we are showing in
dropdown.
--
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-unsubscr...@superset.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
-
To unsubscribe, e-mail: notifications-unsubscr...@superset.apache.org
For additional commands, e-mail: notifications-h...@superset.apache.org
Re: [PR] feat: custom refresh frequency [superset]
Abhishek-kumar-samsung commented on PR #24449: URL: https://github.com/apache/superset/pull/24449#issuecomment-1953466747 Thankyou for reviews @geido I will check and correct the things. -- 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-unsubscr...@superset.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org - To unsubscribe, e-mail: notifications-unsubscr...@superset.apache.org For additional commands, e-mail: notifications-h...@superset.apache.org
Re: [PR] feat: custom refresh frequency [superset]
geido commented on code in PR #24449:
URL: https://github.com/apache/superset/pull/24449#discussion_r1494742341
##
superset-frontend/src/dashboard/components/RefreshIntervalModal.tsx:
##
@@ -91,6 +99,51 @@ class RefreshIntervalModal extends React.PureComponent<
this.setState({
refreshFrequency: value || refreshIntervalOptions[0][0],
});
+
+if (value === -1) {
+ this.setState({
+custom_block: true,
+ });
+} else {
+ this.setState({
+custom_block: false,
+ });
+}
+ }
+
+ onSaveValue(value: number) {
+this.props.onChange(value, this.props.editMode);
+this.modalRef?.current?.close();
+this.props.addSuccessToast(t('Refresh interval saved'));
+ }
+
+ createIntervalOptions(refreshIntervalOptions: [number, string][]) {
+const refresh_options = [];
+if (refreshIntervalOptions.length === 0) {
Review Comment:
```suggestion
if (!refreshIntervalOptions.length) {
```
##
superset-frontend/src/dashboard/components/RefreshIntervalModal.tsx:
##
@@ -91,6 +99,51 @@ class RefreshIntervalModal extends React.PureComponent<
this.setState({
refreshFrequency: value || refreshIntervalOptions[0][0],
});
+
+if (value === -1) {
+ this.setState({
+custom_block: true,
+ });
+} else {
+ this.setState({
+custom_block: false,
+ });
+}
+ }
Review Comment:
```
this.setState({
custom_block: value === -1,
});
```
##
superset-frontend/src/dashboard/components/RefreshIntervalModal.tsx:
##
@@ -111,17 +170,81 @@ class RefreshIntervalModal extends React.PureComponent<
modalTitle={t('Refresh interval')}
modalBody={
-{t('Refresh frequency')}
- ({
-value: option[0],
-label: t(option[1]),
- }))}
- value={refreshFrequency}
- onChange={this.handleFrequencyChange}
- sortComparator={propertyComparator('value')}
-/>
+
+
+{t('Refresh frequency')}
+
+
+
+
+
+
+ {t('HOUR')}
+{' '}
+
+
-{t('Refresh frequency')}
- ({
-value: option[0],
-label: t(option[1]),
- }))}
- value={refreshFrequency}
- onChange={this.handleFrequencyChange}
- sortComparator={propertyComparator('value')}
-/>
+
+
+{t('Refresh frequency')}
+
+
+
+
-{t('Refresh frequency')}
- ({
-value: option[0],
-label: t(option[1]),
- }))}
- value={refreshFrequency}
- onChange={this.handleFrequencyChange}
- sortComparator={propertyComparator('value')}
-/>
+
+
+{t('Refresh frequency')}
+
+
+
+
+
+
+ {t('HOUR')}
+{' '}
+
+ ({
value: i,
label: `${i} ${min_or_sec}`,
}));
}
```
--
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-unsubscr...@superset.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
-
To unsubscribe, e-mail: notifications-unsubscr...@superset.apache.org
For additional commands, e-mail: notifications-h...@superset.apache.org
Re: [PR] feat: custom refresh frequency [superset]
geido commented on PR #24449: URL: https://github.com/apache/superset/pull/24449#issuecomment-1952715404 Hello @Abhishek-kumar-samsung I am observing some behaviours that should be fixed. In the video you can see the following: - When going from an option to another, the initial values that I did set for the custom options are not cleared - When moving from a custom option to a standard option, the values will stay on screen for a short blink and then disappear https://github.com/apache/superset/assets/60598000/cb53640a-b8e0-4580-a22b-a41f662be3bb -- 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-unsubscr...@superset.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org - To unsubscribe, e-mail: notifications-unsubscr...@superset.apache.org For additional commands, e-mail: notifications-h...@superset.apache.org
Re: [PR] feat: custom refresh frequency [superset]
github-actions[bot] commented on PR #24449: URL: https://github.com/apache/superset/pull/24449#issuecomment-1939258090 @geido Ephemeral environment spinning up at http://18.236.100.103:8080. Credentials are `admin`/`admin`. Please allow several minutes for bootstrapping and startup. -- 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-unsubscr...@superset.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org - To unsubscribe, e-mail: notifications-unsubscr...@superset.apache.org For additional commands, e-mail: notifications-h...@superset.apache.org
Re: [PR] feat: custom refresh frequency [superset]
geido commented on PR #24449: URL: https://github.com/apache/superset/pull/24449#issuecomment-1939237154 /testenv up -- 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-unsubscr...@superset.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org - To unsubscribe, e-mail: notifications-unsubscr...@superset.apache.org For additional commands, e-mail: notifications-h...@superset.apache.org
Re: [PR] feat: custom refresh frequency [superset]
github-actions[bot] commented on PR #24449: URL: https://github.com/apache/superset/pull/24449#issuecomment-1936992399 @Abhishek-kumar-samsung Ephemeral environment creation is currently limited to committers. -- 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-unsubscr...@superset.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org - To unsubscribe, e-mail: notifications-unsubscr...@superset.apache.org For additional commands, e-mail: notifications-h...@superset.apache.org
Re: [PR] feat: custom refresh frequency [superset]
Abhishek-kumar-samsung commented on PR #24449: URL: https://github.com/apache/superset/pull/24449#issuecomment-1936992198 /testenv up -- 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-unsubscr...@superset.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org - To unsubscribe, e-mail: notifications-unsubscr...@superset.apache.org For additional commands, e-mail: notifications-h...@superset.apache.org
Re: [PR] feat: custom refresh frequency [superset]
Abhishek-kumar-samsung commented on PR #24449: URL: https://github.com/apache/superset/pull/24449#issuecomment-1936991459 > @Abhishek-kumar-samsung would you please pull master in your branch and push back? Done, now it is updated to latest master branch -- 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-unsubscr...@superset.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org - To unsubscribe, e-mail: notifications-unsubscr...@superset.apache.org For additional commands, e-mail: notifications-h...@superset.apache.org
Re: [PR] feat: custom refresh frequency [superset]
rusackas commented on PR #24449: URL: https://github.com/apache/superset/pull/24449#issuecomment-1936490888 /testenv up -- 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-unsubscr...@superset.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org - To unsubscribe, e-mail: notifications-unsubscr...@superset.apache.org For additional commands, e-mail: notifications-h...@superset.apache.org
Re: [PR] feat: custom refresh frequency [superset]
geido commented on PR #24449: URL: https://github.com/apache/superset/pull/24449#issuecomment-1936327179 @Abhishek-kumar-samsung would you please pull master in your branch and push back? -- 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-unsubscr...@superset.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org - To unsubscribe, e-mail: notifications-unsubscr...@superset.apache.org For additional commands, e-mail: notifications-h...@superset.apache.org
Re: [PR] feat: custom refresh frequency [superset]
geido commented on PR #24449: URL: https://github.com/apache/superset/pull/24449#issuecomment-1936271215 /testenv up -- 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-unsubscr...@superset.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org - To unsubscribe, e-mail: notifications-unsubscr...@superset.apache.org For additional commands, e-mail: notifications-h...@superset.apache.org
Re: [PR] feat: custom refresh frequency [superset]
Abhishek-kumar-samsung commented on PR #24449: URL: https://github.com/apache/superset/pull/24449#issuecomment-1934023295 Hello @rusackas , @geido , @eschutho , i have removed the use of DOM element to hide/show custom view. Now i have made an element custom_block whose state is determining the visibility of custom view. Please 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-unsubscr...@superset.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org - To unsubscribe, e-mail: notifications-unsubscr...@superset.apache.org For additional commands, e-mail: notifications-h...@superset.apache.org
Re: [PR] feat: custom refresh frequency [superset]
geido commented on PR #24449: URL: https://github.com/apache/superset/pull/24449#issuecomment-1930461161 Hello @Abhishek-kumar-samsung just checking in here. Let me know if you need any support to get this to the finish line. Thanks for your contribution! -- 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-unsubscr...@superset.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org - To unsubscribe, e-mail: notifications-unsubscr...@superset.apache.org For additional commands, e-mail: notifications-h...@superset.apache.org
Re: [PR] feat: custom refresh frequency [superset]
Abhishek-kumar-samsung commented on PR #24449: URL: https://github.com/apache/superset/pull/24449#issuecomment-1890656507 @kasiazjc @rusackas pls review, its long time since no reply. -- 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-unsubscr...@superset.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org - To unsubscribe, e-mail: notifications-unsubscr...@superset.apache.org For additional commands, e-mail: notifications-h...@superset.apache.org
Re: [PR] feat: custom refresh frequency [superset]
Abhishek-kumar-samsung commented on PR #24449: URL: https://github.com/apache/superset/pull/24449#issuecomment-1846774428 @kasiazjc hours was normal input text only, it can take any numeric value greater than or equal to 0. Regarding styling of input text, i removed my custom styling and took one classname from other file and put it.(i.e used styling that was already in the code) Please check. -- 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-unsubscr...@superset.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org - To unsubscribe, e-mail: notifications-unsubscr...@superset.apache.org For additional commands, e-mail: notifications-h...@superset.apache.org
Re: [PR] feat: custom refresh frequency [superset]
kasiazjc commented on PR #24449: URL: https://github.com/apache/superset/pull/24449#issuecomment-1766766398 >  This looks great, thank you! One last request - could you also change "hours" just to normal input? I think there doesn't need to be any limit and user can type in any number. And the same with styling - if you could use input field that we already have in the code that would be great :) -- 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-unsubscr...@superset.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org - To unsubscribe, e-mail: notifications-unsubscr...@superset.apache.org For additional commands, e-mail: notifications-h...@superset.apache.org
Re: [PR] feat: custom refresh frequency [superset]
github-actions[bot] commented on PR #24449: URL: https://github.com/apache/superset/pull/24449#issuecomment-1765338361 @yousoph Ephemeral environment spinning up at http://54.212.14.85:8080. Credentials are `admin`/`admin`. Please allow several minutes for bootstrapping and startup. -- 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-unsubscr...@superset.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org - To unsubscribe, e-mail: notifications-unsubscr...@superset.apache.org For additional commands, e-mail: notifications-h...@superset.apache.org
Re: [PR] feat: custom refresh frequency [superset]
yousoph commented on PR #24449: URL: https://github.com/apache/superset/pull/24449#issuecomment-1765333595 /testenv up -- 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-unsubscr...@superset.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org - To unsubscribe, e-mail: notifications-unsubscr...@superset.apache.org For additional commands, e-mail: notifications-h...@superset.apache.org
Re: [PR] feat: custom refresh frequency [superset]
Abhishek-kumar-samsung commented on PR #24449: URL: https://github.com/apache/superset/pull/24449#issuecomment-1763034395 ![Uploading IMG_1487.jpeg…]() -- 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-unsubscr...@superset.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org - To unsubscribe, e-mail: notifications-unsubscr...@superset.apache.org For additional commands, e-mail: notifications-h...@superset.apache.org
Re: [PR] feat: custom refresh frequency [superset]
Abhishek-kumar-samsung commented on PR #24449: URL: https://github.com/apache/superset/pull/24449#issuecomment-1763034637  -- 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-unsubscr...@superset.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org - To unsubscribe, e-mail: notifications-unsubscr...@superset.apache.org For additional commands, e-mail: notifications-h...@superset.apache.org
Re: [PR] feat: custom refresh frequency [superset]
Abhishek-kumar-samsung commented on PR #24449: URL: https://github.com/apache/superset/pull/24449#issuecomment-1763033316 > Sorry for the super late review! Missed this PR. These are not ant design components right? I think single select with predefined list in the dropdown we have in code should be the one used in this for consistency (design and patterns). You can find it in filters for example, and users can also type in some numbers (it just would not accept the new number) @kasiazjc I have made changes as per your suggestion, i have removed and put select dropdown, now one can also filter based on select dropdown options, now it looks exactly similar to earlier dropdowns, i have checked in chrome, edge, firefox, it is similar in all browsers. -- 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-unsubscr...@superset.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org - To unsubscribe, e-mail: notifications-unsubscr...@superset.apache.org For additional commands, e-mail: notifications-h...@superset.apache.org
Re: [PR] feat: custom refresh frequency [superset]
Abhishek-kumar-samsung commented on code in PR #24449:
URL: https://github.com/apache/superset/pull/24449#discussion_r1359476548
##
superset-frontend/src/dashboard/components/RefreshIntervalModal.tsx:
##
@@ -91,6 +91,52 @@ class RefreshIntervalModal extends React.PureComponent<
this.setState({
refreshFrequency: value || refreshIntervalOptions[0][0],
});
+
+const custom_block = document.getElementById('custom_block_view');
Review Comment:
while implementing this, i was not getting how to hide/unhide custom options
from react so i implemented onclick to hide/unhide dom element.
Not sure how can i do without dom element, but i will check and do if
possible.
--
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-unsubscr...@superset.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
-
To unsubscribe, e-mail: notifications-unsubscr...@superset.apache.org
For additional commands, e-mail: notifications-h...@superset.apache.org
Re: [PR] feat: custom refresh frequency [superset]
Abhishek-kumar-samsung commented on code in PR #24449:
URL: https://github.com/apache/superset/pull/24449#discussion_r1359476058
##
superset-frontend/src/dashboard/components/RefreshIntervalModal.tsx:
##
@@ -111,17 +157,89 @@ class RefreshIntervalModal extends React.PureComponent<
modalTitle={t('Refresh interval')}
modalBody={
-{t('Refresh frequency')}
- ({
-value: option[0],
-label: t(option[1]),
- }))}
- value={refreshFrequency}
- onChange={this.handleFrequencyChange}
- sortComparator={propertyComparator('value')}
-/>
+
+
+{t('Refresh frequency')}
+
+
+
+
Re: [PR] feat: custom refresh frequency [superset]
kasiazjc commented on PR #24449: URL: https://github.com/apache/superset/pull/24449#issuecomment-1757536331 > Hi @kasiazjc I added dropdowns in minute and seconds as you have suggested. Instead of select i used input type with datalist. In datalist i have provided options from 0-59, if user puts some other value and submits then it will display error message. > > datalist is looking little different depending on browsers, some previews are as below. > > **edge browser preview:**  > > **firefox browser preview:**  > > **chrome browser preview:**  > > I was having problem taking screenshot with dropdown, so i took screenshot using phone. > > Please check and review @kasiazjc @rusackas Sorry for the super late review! Missed this PR. These are not ant design components right? I think single select with predefined list in the dropdown we have in code should be the one used in this for consistency (design and patterns). You can find it in filters for example, and users can also type in some numbers (it just would not accept the new number) -- 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-unsubscr...@superset.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org - To unsubscribe, e-mail: notifications-unsubscr...@superset.apache.org For additional commands, e-mail: notifications-h...@superset.apache.org
Re: [PR] feat: custom refresh frequency [superset]
github-actions[bot] commented on PR #24449: URL: https://github.com/apache/superset/pull/24449#issuecomment-1756379715 @yousoph Ephemeral environment spinning up at http://52.12.12.69:8080. Credentials are `admin`/`admin`. Please allow several minutes for bootstrapping and startup. -- 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-unsubscr...@superset.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org - To unsubscribe, e-mail: notifications-unsubscr...@superset.apache.org For additional commands, e-mail: notifications-h...@superset.apache.org
Re: [PR] feat: custom refresh frequency [superset]
yousoph commented on PR #24449: URL: https://github.com/apache/superset/pull/24449#issuecomment-1756375585 /testenv up -- 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-unsubscr...@superset.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org - To unsubscribe, e-mail: notifications-unsubscr...@superset.apache.org For additional commands, e-mail: notifications-h...@superset.apache.org
Re: [PR] feat: custom refresh frequency [superset]
eschutho commented on code in PR #24449:
URL: https://github.com/apache/superset/pull/24449#discussion_r1353496153
##
superset-frontend/src/dashboard/components/RefreshIntervalModal.tsx:
##
@@ -91,6 +91,52 @@ class RefreshIntervalModal extends React.PureComponent<
this.setState({
refreshFrequency: value || refreshIntervalOptions[0][0],
});
+
+const custom_block = document.getElementById('custom_block_view');
Review Comment:
is it possible to control/select this element with a ref instead of directly
fetching from the dom?
--
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-unsubscr...@superset.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
-
To unsubscribe, e-mail: notifications-unsubscr...@superset.apache.org
For additional commands, e-mail: notifications-h...@superset.apache.org
Re: [PR] feat: custom refresh frequency [superset]
eschutho commented on code in PR #24449:
URL: https://github.com/apache/superset/pull/24449#discussion_r1353495822
##
superset-frontend/src/dashboard/components/RefreshIntervalModal.tsx:
##
@@ -111,17 +157,89 @@ class RefreshIntervalModal extends React.PureComponent<
modalTitle={t('Refresh interval')}
modalBody={
-{t('Refresh frequency')}
- ({
-value: option[0],
-label: t(option[1]),
- }))}
- value={refreshFrequency}
- onChange={this.handleFrequencyChange}
- sortComparator={propertyComparator('value')}
-/>
+
+
+{t('Refresh frequency')}
+
+
+
+
Re: [PR] feat: custom refresh frequency [superset]
Abhishek-kumar-samsung commented on PR #24449: URL: https://github.com/apache/superset/pull/24449#issuecomment-1752000366 @kasiazjc @rusackas can you pls check the PR, there's been a long time since nobody reviewed it. If any problem is there then pls tell. -- 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-unsubscr...@superset.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org - To unsubscribe, e-mail: notifications-unsubscr...@superset.apache.org For additional commands, e-mail: notifications-h...@superset.apache.org
Re: [PR] feat: custom refresh frequency [superset]
LyleScott commented on PR #24449: URL: https://github.com/apache/superset/pull/24449#issuecomment-1746218018 This would be a welcome addition, thanks for the effort. -- 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-unsubscr...@superset.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org - To unsubscribe, e-mail: notifications-unsubscr...@superset.apache.org For additional commands, e-mail: notifications-h...@superset.apache.org
