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

WIP: Add Dashboard PDF download #2890

Open
wants to merge 2 commits into
base: master
from

Conversation

Projects
None yet
3 participants
@dignati

dignati commented Oct 3, 2018

Heavy WIP implementation of #2861 because I want to get some early feedback and open a discussion. I'm very inexperienced in Angular and Python so any comments are appreciated.

  • UI
    • Download Button
    • Maybe progress indicator in case PDF generation takes a while
  • API call
  • Backend Service
  • Tests

The next step would be to use puppeteer to create a backend service that can be called from the API endpoint to actually generate the PDF. I would be happy to get some suggestions on how this should look. Probably a docker container with puppeteer and a tiny http API that takes a URL and returns a PDF? Or we could start the docker container in the API every time it is needed?

Show outdated Hide outdated client/app/pages/dashboards/dashboard.js
@@ -249,6 +251,11 @@ function DashboardCtrl(
});
};
this.downloadPdf = () => {

This comment has been minimized.

@dignati

dignati Oct 3, 2018

Have to check if this even runs when a click on the download button starts the download anyway.

@dignati

dignati Oct 3, 2018

Have to check if this even runs when a click on the download button starts the download anyway.

api.add_org_resource(QueryFavoriteResource, '/api/queries/<query_id>/favorite', endpoint='query_fovorite')
api.add_org_resource(DashboardFavoriteListResource, '/api/dashboards/favorites', endpoint='dashboard_fovorites')
api.add_org_resource(DashboardFavoriteResource, '/api/dashboards/<object_id>/favorite', endpoint='dashboard_fovorite')
api.add_org_resource(QueryFavoriteListResource, '/api/queries/favorites', endpoint='query_favorites')

This comment has been minimized.

@dignati

dignati Oct 3, 2018

I assume these were typos?

@dignati

dignati Oct 3, 2018

I assume these were typos?

class DashboardPdfDownload(BaseResource):
def get(self, dashboard_slug=None):
"dashboard = get_object_or_404(models.Dashboard.get_by_slug_and_org, dashboard_slug, self.current_org)"

This comment has been minimized.

@kravets-levko

kravets-levko Oct 3, 2018

Collaborator

What is this for?

@kravets-levko

kravets-levko Oct 3, 2018

Collaborator

What is this for?

This comment has been minimized.

@dignati

dignati Oct 3, 2018

Nothing yet, this is copy paste leftover. I assume we need to get the url for the dashboard so that we can pass it on to puppeteer.

@dignati

dignati Oct 3, 2018

Nothing yet, this is copy paste leftover. I assume we need to get the url for the dashboard so that we can pass it on to puppeteer.

self.record_event({
'action': 'download_pdf',
'object_type': 'dashboard',

This comment has been minimized.

@kravets-levko

kravets-levko Oct 3, 2018

Collaborator

I think, it's useful to record also dashboard id here

@kravets-levko

kravets-levko Oct 3, 2018

Collaborator

I think, it's useful to record also dashboard id here

This comment has been minimized.

@dignati

dignati Oct 3, 2018

Will do.

@dignati

dignati Oct 3, 2018

Will do.

@arikfr

This comment has been minimized.

Show comment
Hide comment
@arikfr

arikfr Oct 3, 2018

Member

Maybe progress indicator in case PDF generation takes a while

It will take a while :) Depends on how much resources the machine has and such, but a few seconds at least.

The next step would be to use puppeteer to create a backend service that can be called from the API endpoint to actually generate the PDF. I would be happy to get some suggestions on how this should look. Probably a docker container with puppeteer and a tiny http API that takes a URL and returns a PDF? Or we could start the docker container in the API every time it is needed?

My suggestion is let's make this implementation detail transparent to Redash, by using a configuration where you pass a URL of the snapshot service to Redash. This way the end user can deploy it with Docker, Serverless or whatever.

For our default deployment it will be another Docker service we will add to the Docker Compose configuration.

Member

arikfr commented Oct 3, 2018

Maybe progress indicator in case PDF generation takes a while

It will take a while :) Depends on how much resources the machine has and such, but a few seconds at least.

The next step would be to use puppeteer to create a backend service that can be called from the API endpoint to actually generate the PDF. I would be happy to get some suggestions on how this should look. Probably a docker container with puppeteer and a tiny http API that takes a URL and returns a PDF? Or we could start the docker container in the API every time it is needed?

My suggestion is let's make this implementation detail transparent to Redash, by using a configuration where you pass a URL of the snapshot service to Redash. This way the end user can deploy it with Docker, Serverless or whatever.

For our default deployment it will be another Docker service we will add to the Docker Compose configuration.

@dignati

This comment has been minimized.

Show comment
Hide comment
@dignati

dignati Oct 3, 2018

It will take a while :) Depends on how much resources the machine has and such, but a few seconds at least.
Yeah, seems likely. This might be outside of my UI Design capabilities though.

My suggestion is let's make this implementation detail transparent to Redash, by using a configuration where you pass a URL of the snapshot service to Redash. This way the end user can deploy it with Docker, Serverless or whatever.

Okay, this sounds good. Should the new service be developed here, in-tree or in an extra repo?

dignati commented Oct 3, 2018

It will take a while :) Depends on how much resources the machine has and such, but a few seconds at least.
Yeah, seems likely. This might be outside of my UI Design capabilities though.

My suggestion is let's make this implementation detail transparent to Redash, by using a configuration where you pass a URL of the snapshot service to Redash. This way the end user can deploy it with Docker, Serverless or whatever.

Okay, this sounds good. Should the new service be developed here, in-tree or in an extra repo?

@arikfr

This comment has been minimized.

Show comment
Hide comment
@arikfr

arikfr Oct 3, 2018

Member

Okay, this sounds good. Should the new service be developed here, in-tree or in an extra repo?

I think a separate repo makes sense. I will open one tomorrow.

Member

arikfr commented Oct 3, 2018

Okay, this sounds good. Should the new service be developed here, in-tree or in an extra repo?

I think a separate repo makes sense. I will open one tomorrow.

@arikfr

This comment has been minimized.

Show comment
Hide comment
@arikfr

arikfr Oct 4, 2018

Member

@dignati I opened a new repo for the snapshot service: https://github.com/getredash/snap. I also uploaded there the codebase we currently use to take snapshots of visualizations for the Slack bot as reference.

Let me know if I can help with anything else.

Member

arikfr commented Oct 4, 2018

@dignati I opened a new repo for the snapshot service: https://github.com/getredash/snap. I also uploaded there the codebase we currently use to take snapshots of visualizations for the Slack bot as reference.

Let me know if I can help with anything else.

@dignati

This comment has been minimized.

Show comment
Hide comment
@dignati

dignati Oct 8, 2018

The dashboard API endpoint does not seem to accept a ?api_key= parameter like the query endpoint does. This needs to be added first so that the snap service can access the dashboard.

It is unclear to me how the snap service should authorize itself to the redash API to get access to the dashboard. There is a /embed endpoint for queries that accepts a api_key as URL parameter which works fine. The /public endpoint that provides something similar for dashboards however requires the dashboard to be public. Maybe we can extend /embed to support dashboards?

dignati commented Oct 8, 2018

The dashboard API endpoint does not seem to accept a ?api_key= parameter like the query endpoint does. This needs to be added first so that the snap service can access the dashboard.

It is unclear to me how the snap service should authorize itself to the redash API to get access to the dashboard. There is a /embed endpoint for queries that accepts a api_key as URL parameter which works fine. The /public endpoint that provides something similar for dashboards however requires the dashboard to be public. Maybe we can extend /embed to support dashboards?

@arikfr

This comment has been minimized.

Show comment
Hide comment
@arikfr

arikfr Oct 10, 2018

Member

It is unclear to me how the snap service should authorize itself to the redash API to get access to the dashboard. There is a /embed endpoint for queries that accepts a api_key as URL parameter which works fine. The /public endpoint that provides something similar for dashboards however requires the dashboard to be public. Maybe we can extend /embed to support dashboards?

It's an open question for now :) The best solution will be to have some temporary API key that gives access to this dashboard. For now you can develop this using a user API key, until we figure out a better way.

Member

arikfr commented Oct 10, 2018

It is unclear to me how the snap service should authorize itself to the redash API to get access to the dashboard. There is a /embed endpoint for queries that accepts a api_key as URL parameter which works fine. The /public endpoint that provides something similar for dashboards however requires the dashboard to be public. Maybe we can extend /embed to support dashboards?

It's an open question for now :) The best solution will be to have some temporary API key that gives access to this dashboard. For now you can develop this using a user API key, until we figure out a better way.

@dignati

This comment has been minimized.

Show comment
Hide comment
@dignati

dignati Oct 13, 2018

@arikfr Okay, I'm having trouble using the User API key. Do I understand correctly that the snap service should be able to set HTTP Header Authorization to Key $user_api_key and then "see" the dashboard page?

dignati commented Oct 13, 2018

@arikfr Okay, I'm having trouble using the User API key. Do I understand correctly that the snap service should be able to set HTTP Header Authorization to Key $user_api_key and then "see" the dashboard page?

@arikfr

This comment has been minimized.

Show comment
Hide comment
@arikfr

arikfr Oct 14, 2018

Member

Okay, I'm having trouble using the User API key.

What is the issue?

Do I understand correctly that the snap service should be able to set HTTP Header Authorization to Key $user_api_key and then "see" the dashboard page?

Yes.

Member

arikfr commented Oct 14, 2018

Okay, I'm having trouble using the User API key.

What is the issue?

Do I understand correctly that the snap service should be able to set HTTP Header Authorization to Key $user_api_key and then "see" the dashboard page?

Yes.

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