airbyte-ci: Switch to prod pypi #34606
Conversation
|
The latest updates on your projects. Learn more about Vercel for Git ↗︎ 1 Ignored Deployment
|
There was a problem hiding this comment.
@flash1293 are we still using the test PyPi when calling publish with the --pre-release option?
| #### Python registry publishing | ||
|
|
||
| If `remoteRegistries.pypi.enabled` in the connector metadata is set to `true`, the connector will be published to the python registry. | ||
| To do so, the `PYTHON_REGISTRY_TOKEN` and `PYTHON_REGISTRY_URL` env vars are used to authenticate with the registry and publish the connector. |
There was a problem hiding this comment.
| To do so, the `PYTHON_REGISTRY_TOKEN` and `PYTHON_REGISTRY_URL` env vars are used to authenticate with the registry and publish the connector. | |
| To do so, the `--python-registry-token` and `--python-registry-url` options are used to authenticate with the registry and publish the connector. |
There was a problem hiding this comment.
These are only available on the poetry publish command, which is not used for connectors. For connectors you actually have to use the env variables.
There was a problem hiding this comment.
Oh I did not notice this. I'd prefer to have the same option on the connector side 😄
There was a problem hiding this comment.
Added these options (renamed the registry option to python registry for consistency)
| | `--publish-name` | False | | | The name of the package. Not required for poetry packages that define it in the `pyproject.toml` file | | ||
| | `--publish-version` | False | | | The version of the package. Not required for poetry packages that define it in the `pyproject.toml` file | | ||
| | `--python-registry-token` | True | | PYTHON_REGISTRY_TOKEN | The API token to authenticate with the registry. For pypi, the `pypi-` prefix needs to be specified | | ||
| | `--registry-url` | False | https://upload.pypi.org/legacy/ | PYTHON_REGISTRY_URL | The python registry to publish to. Defaults to main pypi | |
There was a problem hiding this comment.
I'm wondering where the default URL should live in. I'd say it should be at the airbyte-ci CLI level, which would mean the PYTHON_REGISTRY_URL env var does not need to be set on main-release
There was a problem hiding this comment.
Definitely on the airbyte-ci level, but IMHO it makes sense here as well for local usage.
airbyte-ci/connectors/pipelines/pipelines/airbyte_ci/steps/python_registry.py
Show resolved
Hide resolved
No, it's going into regular pypi as a dev release (e.g. 
test pypi is more of a development thing for publishing pipelines. |
|
@alafanechere could addressed your points, could you take another look? |
| python_registry_token: Optional[str] = None, | ||
| python_registry_url: Optional[str] = None, |
There was a problem hiding this comment.
Can we add a validation on init that these keyword arguments are not None when self.connector.language in [ConnectorLanguage.Python, ConnectorLanguage.LowCode]?
There was a problem hiding this comment.
added a check to the step - if the python publish logic is hit, it will fail with a descriptive error in case the registry or token isn't set properly.
octavia-squidington-iii
added
area/connectors
|
Published airbyte-lib and a test connector: |
This PR switches the python registry publish logic over to prod pypi:
For this to work in CI, the
PYPI_TOKENGithub secret needs to be updated (token is generated already).I tested manually with a poetry and setup.py package and was able to publish without issues: