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

Check for path==None #227

Closed
wants to merge 1 commit into from

Conversation

tpvasconcelos
Copy link

@tpvasconcelos tpvasconcelos commented Nov 15, 2024

Closes #226

@tpvasconcelos tpvasconcelos marked this pull request as ready for review November 15, 2024 23:50
@gvwilson gvwilson requested a review from ayjayt November 18, 2024 13:56
@gvwilson gvwilson added community community contribution fix fixes something broken P1 needs immediate attention labels Nov 18, 2024
@ayjayt
Copy link
Collaborator

ayjayt commented Nov 18, 2024

Hi! This was already fixed and merged by #224

  • the real issue here is that they don't have chrome or chromium installed! But that fix explicitly checks for that and then explicitly raises an error.

Need to push a minor revision! Thanks!

@gvwilson @tpvasconcelos

@ayjayt ayjayt closed this Nov 18, 2024
@tpvasconcelos tpvasconcelos deleted the 226-browser-path branch November 18, 2024 23:26
@tpvasconcelos
Copy link
Author

@ayjayt Thanks a lot for the feedback!

Just a question: wouldn't this warrant a major version bump (or at least minor) since it's technically a breaking change? This will break all running instances that do not have chromium installed, which will be most servers and CI systems.

Thanks again!

@ayjayt
Copy link
Collaborator

ayjayt commented Nov 19, 2024

Yeah thats what we're considering. You really can't treat it like a pure python dependency change because the managers don't pull Chrome automatically. I think tomorrow they'll yank v0.4.x and release it as v1.0.0. i apologize for the inconvenience, I'm not a plotly employee but I am the lead developer on the project and I should have anticipated this.

(Yank won't delete the version, just make it not automatically downloaded)

@tpvasconcelos
Copy link
Author

Thanks for clarifying and no worries at all from my side!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
community community contribution fix fixes something broken P1 needs immediate attention
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Upgrade kaleido 0.2.1 -> 0.4.1 is causing errors in CI
3 participants