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

Feature/md 7041 publish python package #34

Merged
merged 3 commits into from
Nov 2, 2023

Conversation

marns93
Copy link
Contributor

@marns93 marns93 commented Oct 27, 2023

Added possibility to publish Python package to PyPI, see #32.

There is another PR to add the credentials: https://github.com/moneymeets/moneymeets-pulumi/pull/483, which I already applied.

We agreed on the following versioning schema:
locally: 1.1+SNAPSHOT
published: 1.0.202310461046+gabcde

The workflow can be tested by updating the branch from master to feature/MD-7041-publish-python-package in publish.yml.
Furthermore, there is a to-do item in pyproject.toml. @catcombo Please clarify and update this.

@catcombo
Copy link
Contributor

I've updated classifiers according to the list of classifiers. I also preselected MIT license. We can discuss it in the IT daily if we want to choose another one.

@zyv
Copy link
Contributor

zyv commented Oct 27, 2023

I thinks MIT is the best for us. Thank you!

pyproject.toml Outdated Show resolved Hide resolved
Copy link
Contributor

@catcombo catcombo left a comment

Choose a reason for hiding this comment

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

I've tested building package locally and haven't found any problems with the package content or with the build process.

@marns93
Copy link
Contributor Author

marns93 commented Oct 27, 2023

/rebase

@marns93 marns93 marked this pull request as ready for review October 27, 2023 14:04
@mergealot mergealot force-pushed the feature/MD-7041-publish-python-package branch from f13fe01 to 9e8d4ae Compare October 27, 2023 14:04
@catcombo
Copy link
Contributor

@felix11h Do you need more time to review this PR or can I merge it?

Copy link
Contributor

@felix11h felix11h left a comment

Choose a reason for hiding this comment

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

Thank you for waiting for my review, @catcombo. I have approved the changes, there's nothing I see that would speak against merging as is.

I did leave some additional comments, which are in part for my own understanding. Maybe this is still something we can think and potentially update or change in a second step.

.github/workflows/publish.yml Show resolved Hide resolved
description = ""
authors = []
version = "1.1+SNAPSHOT"
description = "YouTrack SDK"
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
description = "YouTrack SDK"
description = "YouTrack API SDK"

To make it consistent with the description of the GitHub repository?

Copy link
Contributor

Choose a reason for hiding this comment

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

I'm not sure that YouTrack API SDK is the correct description. For example, boto3 is just AWS SDK for Python, not AWS API SDK. So maybe we should update repository description instead of package description.

pyproject.toml Show resolved Hide resolved
@catcombo catcombo merged commit 8dc0406 into master Nov 2, 2023
5 checks passed
@catcombo catcombo deleted the feature/MD-7041-publish-python-package branch November 2, 2023 06:49
@catcombo
Copy link
Contributor

catcombo commented Nov 2, 2023

There are 2 problems:

  1. Publishing failed because PyPI server doesn't support local versions (+git-hash). We'll discuss the solution to this in the ticket or in the daily.
  2. I noticed that the publish workflow started in parallel with the CI workflow. Is it possible to start publishing only after CI has been successfully completed?

@marns93
Copy link
Contributor Author

marns93 commented Nov 2, 2023

There are 2 problems:

  1. Publishing failed because PyPI server doesn't support local versions (+git-hash). We'll discuss the solution to this in the ticket or in the daily.

:( Okay, let's discuss it again.

  1. I noticed that the publish workflow started in parallel with the CI workflow. Is it possible to start publishing only after CI has been successfully completed?

We discussed this and the point from @felix11h was that CI workflow will be successful in feature branches, otherwise we can't merge to master. So it is okay to run in parallel. Otherwise, we could use the way with a composite action and add a condition, or we can check the trigger types.

@catcombo
Copy link
Contributor

catcombo commented Nov 2, 2023

We discussed this and the point from @felix11h was that CI workflow will be successful in feature branches, otherwise we can't merge to master. So it is okay to run in parallel. Otherwise, we could use the way with a composite action and add a condition, or we can check the trigger types.

Thanks for the explanation. This makes sense.

@catcombo
Copy link
Contributor

catcombo commented Nov 2, 2023

:( Okay, let's discuss it again.

We've decided in the IT daily to remove local version and deploy a package.

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

Successfully merging this pull request may close these issues.

4 participants