search

bluet / proxybroker2

The New (auto rotate) Proxy [Finder | Checker | Server]. HTTP(S) & SOCKS :performing_arts:
Apache License 2.0
748 stars 113 forks source link

Poetry as the only package manager? #105

Open bluet opened 2 years ago

bluet commented 2 years ago

Follow-up discussion of https://github.com/bluet/proxybroker2/pull/92

@hms5232 got your point :ok_hand:

@afunTW I don't think we can/should generate/update requirements.txt in CI pipeline. CI shouldn't make changes to the source repo, isn't it?
IIRC, the export command of Poetry can be used to generate requirements.txt. So, in practice, we can either ask developers to remember to run the command every time they update dependency, or just document the generation command in README.md for users who really want to use requirements.txt. :rofl:
Not sure if there are other ways.

bluet commented 2 years ago

The pre-commit git hook might be another way. Call the Poetry export command every time they make new commits.
But it might require an existing working poetry installed in $PATH. Just not sure if this should be a hard requirement.

hms5232 commented 2 years ago

I think we only use Poetry in this project.

For example, Javascript/Node has three package manager at least, but project will not maintain package-lock.json, yarn.lock and pnpm-lock.yaml. They will choose one as their common tool.

Pip is good and has built-in in Python, but it can't control dependencies exactly (This is a disaster when you have many projects use different dependency version in same environment/computer).

afunTW commented 2 years ago

Understood and agree that CI shouldn't make changes. But I would like to separate into two topics here

  1. Support multiple package manager or not?
  2. Maintain the build-in requirements.txt or not?

Considering the discussion above and the phase of the project right now, supporting only one package manager is the consensus between @bluet, @hms5232, and me.

On the other hand, the built-in package manager always catches my eye on it. I believe the support of requirements.txt is the basic developer-friendly feature.

For example, Javascript/Node has three package manager at least, but project will not maintain package-lock.json, yarn.lock and pnpm-lock.yaml. They will choose one as their common tool.

Actually, the meaning of requirements.txt is like package.json rather than the lock file to me.

hms5232 commented 2 years ago

Understood and agree that CI shouldn't make changes. But I would like to separate into two topics here

1. Support multiple package manager or not?

2. Maintain the build-in requirements.txt or not?

Considering the discussion above and the phase of the project right now, supporting only one package manager is the consensus between @bluet, @hms5232, and me.

On the other hand, the built-in package manager always catches my eye on it. I believe the support of requirements.txt is the basic developer-friendly feature.

For example, Javascript/Node has three package manager at least, but project will not maintain package-lock.json, yarn.lock and pnpm-lock.yaml. They will choose one as their common tool.

Actually, the meaning of requirements.txt is like package.json rather than the lock file to me.

This is one of downside of pip. The requirements.txt looks like:

urllib3==1.26.7
virtualenv==20.10.0
virtualenv-clone==0.5.4

That show the package name and version but not include/point out their dependency graph. So I think requirements.txt is between package.json and lock file. How about you?

The PEP 582 is still in draft, so we should consider that how to keep dependency fit those package but not effect other package in environment (or in reverse, the environment effect project dependencies).

In conclusion, Poetry or Pipenv is good to reach it. Nevertheless, Pipenv has some negative point so I no longer use it. It is my belief:

  1. We should only support one package manager (current is Poetry).
  2. We should maintain both pyproject.toml (PEP 621) and poetry.lock but not include requirements.txt.

By the way, if we want to migrate to PEP 582 in the future, I recommend pyflow or pdm.

afunTW commented 2 years ago

So I think requirements.txt is between package.json and lock file. How about you?

Agree, package.json event composes some other functionality.

We should only support one package manager (current is Poetry).

I believe we are on the same page

We should maintain both pyproject.toml (PEP 621) and poetry.lock but not include requirements.txt

What I am only concerned about is the support on requirements.txt. But letting it out of our maintenance scope is fine for our first milestone. Considering the multiple Python supports, this is not a well fit for this project indeed as well.

hms5232 commented 2 years ago

So I think requirements.txt is between package.json and lock file. How about you?

Agree, package.json event composes some other functionality.

We should only support one package manager (current is Poetry).

I believe we are on the same page

We should maintain both pyproject.toml (PEP 621) and poetry.lock but not include requirements.txt

What I am only concerned about is the support on requirements.txt. But letting it out of our maintenance scope is fine for our first milestone. Considering the multiple Python supports, this is not a well fit for this project indeed as well.

Yes, Poetry has Python version check. And they released 1.2.0 recently. I will make a PR to upgrade CI setting after I use it for a while.

bluet commented 2 years ago

hi @hms5232 , still waiting for your PR hahaha

hms5232 commented 2 years ago

hi @hms5232 , still waiting for your PR hahaha

A few weeks ago I found a bug at 1.2.1 and report to poetry, they has fixed it at main branch, but still not release in new version. I'm waiting for this, lol.

Poetry 1.2 have many features and some of them are incompatible with 1.1. I am on the road to learn how to apply them. 🚀

bluet commented 2 years ago

@hms5232 ok cool, sounds great

hms5232 commented 1 year ago

@hms5232 ok cool, sounds great

Update~ I trace the bug reported issue and notice that Poetry seems that plant to release the fixed at version 1.3. Should we wait for 1.3 or just upgrade pipeline to 1.2 first?

bluet commented 1 year ago

@hms5232 If 1.3 might take a while to release, maybe we should go for 1.2 first.

hms5232 commented 1 year ago

@hms5232 If 1.3 might take a while to release, maybe we should go for 1.2 first.

I'm not sure release schedule whether is delay. The 1.3 marked "Past due by about 1 month" 😄 but 99% completed.

I plan to make a update to go to Poetry 1.2 next week if 1.3 can not release this week. 💪

bluet commented 1 year ago

Sounds great

hms5232 commented 1 year ago

Surprise! 1.3 has released this morning 🎉 . I'm using this version. If everything okay, I will send a PR to bump Poetry version in action pipeline.

bluet commented 1 year ago

Are we ready to be Poetry-only? @hms5232 @afunTW @vincentinttsh @ziloka

ref:

bluet commented 1 year ago

might also need to update README too if we do.

hms5232 commented 1 year ago

Are we ready to be Poetry-only? @hms5232 @afunTW @vincentinttsh @ziloka

As here saying, delete the requirements/, there will no other requirements.txt file exists. Therefore, I vote yes.

might also need to update README too if we do.

Maybe add a develop chapter in README or a CONTRIBUTING.md?

bluet commented 1 year ago

@hms5232 go! go! power rangers!

bluet commented 1 year ago

Seems that we have everything settled and can close the issue?