Skip to content

build: Version bump dx to 0.1.0#86

Merged
jnumainville merged 1 commit intomainfrom
jnumainville-patch-1
Oct 26, 2023
Merged

build: Version bump dx to 0.1.0#86
jnumainville merged 1 commit intomainfrom
jnumainville-patch-1

Conversation

@jnumainville
Copy link
Copy Markdown
Collaborator

Version bump dx to 0.1.0

@mofojed @devinrsmith Are the tokens set properly? I don't think we've pushed a version in this repo yet

@jnumainville jnumainville requested a review from mofojed October 10, 2023 14:20
@jnumainville jnumainville changed the title Version bump dx to 0.1.0 build: Version bump dx to 0.1.0 Oct 10, 2023
@mofojed
Copy link
Copy Markdown
Member

mofojed commented Oct 10, 2023

@jnumainville no not yet, we still need the PYPI_PLOTLY_EXPRESS_TOKEN to be set; though perhaps it'll just be DEEPHAVEN_PYPI_TOKEN instead, and we need to update the token the action uses.
@devinrsmith Can you expose that token to this repo?

@devinrsmith
Copy link
Copy Markdown
Member

It's recommended to scope pypi tokens to specific packages instead of the whole user account. Can we get a list of all the PyPi packages that we'll need tokens for?

@jnumainville
Copy link
Copy Markdown
Collaborator Author

@devinrsmith at the moment: plotly-express, plotly, json, matplotlib
ideally in the form PYPI_PLOTLY_EXPRESS_TOKEN, PYPI_PLOTLY_TOKEN, PYPI_JSON_TOKEN, PYPI_MATPLOTLIB_TOKEN

@devinrsmith
Copy link
Copy Markdown
Member

I'm going to suggest setting up via:

https://blog.pypi.org/posts/2023-04-20-introducing-trusted-publishers/
https://docs.github.com/en/actions/deployment/security-hardening-your-deployments/about-security-hardening-with-openid-connect

This attaches the credentials at the PyPi project level and doesn't need user-level tokens. I've set up trusted publishing for deephaven-plugin-matplotlib and deephaven-plugin-json against this repo and release-python-package.yml.

@jnumainville
Copy link
Copy Markdown
Collaborator Author

I've set up plotly-express for trusted publishing. Changes will be made in #84 for full support

Copy link
Copy Markdown
Member

@mofojed mofojed left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pretty sure we need #84 to merge first, so marking this one as needing changes until that merges.

@jnumainville jnumainville merged commit 790b5b7 into main Oct 26, 2023
@jnumainville jnumainville deleted the jnumainville-patch-1 branch October 26, 2023 14:42
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

Successfully merging this pull request may close these issues.

3 participants