Skip to content
New issue

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

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

Already on GitHub? Sign in to your account

Upload performance reports #281

Open
fjetter opened this issue Aug 26, 2022 · 15 comments
Open

Upload performance reports #281

fjetter opened this issue Aug 26, 2022 · 15 comments

Comments

@fjetter
Copy link
Member

fjetter commented Aug 26, 2022

I'm wondering if there is a reason why we're not automatically uploading performance reports.

using coiled.performance_reports we should have them uploaded automatically and persisted, shouldn't we?

Having perf reports is useful to debug regressions.

@ian-r-rose
Copy link
Contributor

I agree that it would be very nice to have these. Unfortunately, coiled.performance_reports isn't very well maintained, and caps the maximum number at ~5-10. In order to make it useful here, we'd need some work from the platform team.

@ncclementi had some WIP for manually uploading them to s3, but it went stale. I don't think there are any major barriers to doing it (I've even built a field into the benchmark schema for including a URL to a performance report, currently un-populated)

@ncclementi
Copy link
Contributor

@fjetter we had something working, sort of, but the main problem was to generate a link to be able to easy find the reports in S3. Here is a summary of that discussion and work #107 (comment)

@ian-r-rose
Copy link
Contributor

the main problem was to generate a link to be able to easy find the reports in S3.

A way forward there is to use the performance_report_url in the benchmark db and report it in both the static site and the results of detect_regressions.py

@fjetter
Copy link
Member Author

fjetter commented Aug 26, 2022

Unfortunately, coiled.performance_reports isn't very well maintained,

I see... too bad. If this is just about a max limit, we may be able to figure this out.

Just in case, I opened #282

@fjetter
Copy link
Member Author

fjetter commented Aug 26, 2022

A way forward there is to use the performance_report_url in the benchmark db and report it in both the static site and the results of detect_regressions.py

I'd be OK with this as well but we should check in with platform to see how much work it is to get this working on their side. If that's a couple of hours, it's probably better spent there than for our specialized use case.

@dchudz
Copy link
Collaborator

dchudz commented Aug 31, 2022

I think you should be fine to use Coiled performance reports. I'm not aware of anything that would block that.

There is no limit in how many reports you can make.

@dchudz
Copy link
Collaborator

dchudz commented Aug 31, 2022

Oh, I see, the older ones are unavailable once you go over 5.

@dchudz
Copy link
Collaborator

dchudz commented Aug 31, 2022

Discussing here: https://github.com/coiled/platform/issues/57

@dchudz
Copy link
Collaborator

dchudz commented Sep 1, 2022

In the next deploy you'll be unlimited in how many performance reports are accessible with the links. They remain accessible for 2 months.

Note that even now there won't be any errors if you try to create more than the limit, so you're currently completely unblocked in implementing this change in your CI.

@dchudz
Copy link
Collaborator

dchudz commented Sep 17, 2022

I believe any platform-related blockers were removed a couple weeks ago.

Doing this should be pretty quick and easy for you now, right?

@fjetter
Copy link
Member Author

fjetter commented Oct 3, 2022

I believe any platform-related blockers were removed a couple weeks ago.

Doing this should be pretty quick and easy for you now, right?

We were still encountering an issue because the coiled client rejects uploads of larger files than 10MB

@dchudz
Copy link
Collaborator

dchudz commented Oct 6, 2022

@fjetter Is there an issue or the size blocking problem?

@fjetter
Copy link
Member Author

fjetter commented Oct 7, 2022

Is there an issue or the size blocking problem?

The size limit is the problem.

@dchudz
Copy link
Collaborator

dchudz commented Oct 7, 2022

@fjetter is there a github issue for that? It'd help to have a little more info about the problem.

(What sizes are you hitting, etc.?)

I'm sure we can bump the size limit a bit but it would also be nice to work together a bit to understand what solution makes sense for large performance reports.

All that is back and forth we can do in an issue, right?

@fjetter
Copy link
Member Author

fjetter commented Oct 7, 2022

All that is back and forth we can do in an issue, right?

Of course, sorry. I was meaning to open one a while ago but didn't get to it, yet. I'll open one shortly

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

4 participants