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

CI: Simplifying benchmarks call #2451

Merged
merged 8 commits into from
Oct 12, 2020
Merged

CI: Simplifying benchmarks call #2451

merged 8 commits into from
Oct 12, 2020

Conversation

datapythonista
Copy link
Contributor

Just removing the two files used for calling the benchmarks, and implementing what they do directly in the github actions config.

@datapythonista datapythonista added refactor Issues or PRs related to refactoring the codebase ci Continuous Integration issues or PRs labels Oct 6, 2020
@datapythonista datapythonista changed the title CI: Simplifying benchmarks call WIP: Simplifying benchmarks call Oct 6, 2020
@datapythonista datapythonista changed the title WIP: Simplifying benchmarks call CI: Simplifying benchmarks call Oct 7, 2020
@jreback jreback added this to the Next Feature Release milestone Oct 7, 2020
@jreback
Copy link
Contributor

jreback commented Oct 7, 2020

i don't think we should do this just in the github action, rather call the script as otherwise its very hard to debug things.

@datapythonista
Copy link
Contributor Author

i don't think we should do this just in the github action, rather call the script as otherwise its very hard to debug things.

My experience is exactly the opposite. There were some problems with the benchmarks in one PR, and it was quite complicated to understand what we were executing, with 3 files involved. I think this makes it very clear what it's being executed, that's why I made that change.

Not sure what you mean by debug. The only advantage of having a separate script (I don't see any advantage for having two scripts as we do now) would be if we wanted to run the script locally. But if we wanted to do that (I don't think we will in practice), locally just asv is needed.

Can you expand on what is hard to debug with this change for you?

@jreback
Copy link
Contributor

jreback commented Oct 7, 2020

Not sure what you mean by debug. The only advantage of having a separate script (I don't see any advantage for having two scripts as we do now) would be if we wanted to run the script locally. But if we wanted to do that (I don't think we will in practice), locally just asv is needed.

so how does one debug the errors on the benchmarks now? it is totally non-obvious how to run a github action (impossible in fact) locally.

if its 'just' asv.....then that should be the github action. is this the case?

you have to always be able to repro locally, the same way as the CI otherwise debugging is impossible.

@jreback
Copy link
Contributor

jreback commented Oct 12, 2020

kk thanks @datapythonista this looks good. ideally should have a small section in the docs about this, but i don't think really a big deal.

@datapythonista
Copy link
Contributor Author

/azp run

@azure-pipelines
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@jreback jreback merged commit 46e6dff into ibis-project:master Oct 12, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
ci Continuous Integration issues or PRs refactor Issues or PRs related to refactoring the codebase
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants