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

[DISCUSS] Do we want to keep dev tests as strict filter for Ecosystem member #448

Closed
3 tasks done
mickahell opened this issue Jul 13, 2023 · 7 comments · Fixed by #461
Closed
3 tasks done

[DISCUSS] Do we want to keep dev tests as strict filter for Ecosystem member #448

mickahell opened this issue Jul 13, 2023 · 7 comments · Fixed by #461
Assignees
Labels
enhancement New feature or request help wanted Extra attention is needed question Further information is requested

Comments

@mickahell
Copy link
Collaborator

mickahell commented Jul 13, 2023

Conclusion of the discussion

We only keep the standard check as struct filter and change dev and stable as informational.
About the requirement problem, it was part solved in the PR #447, then make dev and stable optional gonna solve the next blocking part

TODO:

  • Dev wf as informational
  • Stable wf as informational
  • Requirement problem

Today's situation

Today when a project is submitted 3 checks are running :

  • Stable check -> force install the latest version of qiskit-terra
  • Dev check -> install the dev version of qiskit-terra
  • Standard check -> just run the test with the original requirements

If we are agree than stable and standard check are mandatory, what about the dev check ?
Initially, we created this check because of the fast development of terra and the breaking changes problem qiskit could have. And so, it was a way to make sure the project not gonna break with the next release.

Positif

Having dev check allows to make sure the project is working well with at least 2 versions of terra, the actual and the next one. Who are pretty cool and anticipate the breaking changes and the fast development of terra.

Negatif

If the project doesn't pass the next release, what can do the maintainer of the project ?
From my knowledge we don't have documentation of development release or even for pre-release. So how can we help maintainer to make their project pass ?

Other ⚠️

Each Qiskit version need a specific version of terra and so we always have these logs during the setup of qiskit-terra dev version :

ecosystem INFO ERROR: pip's dependency resolver does not currently take into account all the packages that are installed. This behaviour is the source of the following dependency conflicts.
ecosystem INFO qiskit 0.43.0 requires qiskit-terra==0.24.0, but you have qiskit-terra 0.25.0 which is incompatible.

We choose to not check ERROR logs during the setup phase and so this doesn't break the process but it's pretty bad to have ERROR log as normal situation.

Conclusion

My point of view would be to still have the dev check during submission but not as mandatory, as informational.
If stable and standard check passed. The submission would be succeeded.
If the dev check doesn't passed, an information comment would be post and notify the maintainer that its submission passed but it not gonna pass for the next release and so he gonna need to update its project when the next release gonna be launch.

Of course, that's only my understanding of the situation. And my point of view maybe doesn't work with the vision Qiskit maintainers have of the Ecosystem.

@mickahell mickahell added enhancement New feature or request help wanted Extra attention is needed question Further information is requested labels Jul 13, 2023
@mickahell
Copy link
Collaborator Author

@frankharkins @Eric-Arellano I would like/need to have your point of view here please :)

@1ucian0
Copy link
Member

1ucian0 commented Jul 14, 2023

If the project doesn't pass the next release, what can do the maintainer of the project ?

Update their code. This means that, in 3 months of less, it will stop working with stable.

we don't have documentation of development release or even for pre-release

True. But our current main is pre-release. You could install from main and fix it.

Ideally, we could have a "warning" when deprecation warnings show up. The warning should tell the user how to fix their code.

Each Qiskit version need a specific version of terra and so we always have these logs during the setup of qiskit-terra dev version

I think this is easy to fix, by installing qiskit before installing qiskit-terra.

My point of view would be to still have the dev check during submission but not as mandatory, as informational.

I'm good with it. Even stable can be information, as the "promise" of the member only applies to standard check.

@mickahell
Copy link
Collaborator Author

Ideally, we could have a "warning" when deprecation warnings show up. The warning should tell the user how to fix their code.

Indeed, I forget that this is the way to do before changing feature. And Ecosystem already track the warning and depreciation logs during check.

Each Qiskit version need a specific version of terra and so we always have these logs during the setup of qiskit-terra dev version

I think this is easy to fix, by installing qiskit before installing qiskit-terra.

Hmm, it's already what we are doing. We first install the project requirements (qiskit and other stuffs the project need) and then we install the pre-release version of qiskit-terra. And we have the error logs because qiskit need the actual release not the release of the main branch of qiskit-terra (main branch of qiskit repo also point to the actual release of qiskit-terra).

Maybe I don't see what you mean @1ucian0 🤔 ?

My point of view would be to still have the dev check during submission but not as mandatory, as informational.

I'm good with it. Even stable can be information, as the "promise" of the member only applies to standard check.

Cool :)
Don't we want to force members to at least when their project is submitted to be able to work with the actual release ?

@1ucian0
Copy link
Member

1ucian0 commented Jul 15, 2023

I think this is easy to fix, by installing qiskit before installing qiskit-terra.

Hmm, it's already what we are doing.

oops. I mean uninstalling qiskit before installing qiskit-terra.

Don't we want to force members to at least when their project is submitted to be able to work with the actual release ?

This might be a too-high bar, given the current state of the ecosystem. Take mitiq for example, a well maintained project that compatible-release-pin qiskit (using ~=) and always updates to stable with some delay. Additionally, things might get old during our review time to add them. And finally, in the near future qiskit might have more than one stable supported at the same time.

But I dont have a strong opinion, just my two cents :)

@mickahell
Copy link
Collaborator Author

I think this is easy to fix, by installing qiskit before installing qiskit-terra.

Hmm, it's already what we are doing.

oops. I mean uninstalling qiskit before installing qiskit-terra.

Oh ok, I'll test that.

Don't we want to force members to at least when their project is submitted to be able to work with the actual release ?

This might be a too-high bar, given the current state of the ecosystem. Take mitiq for example, a well maintained project that compatible-release-pin qiskit (using ~=) and always updates to stable with some delay. Additionally, things might get old during our review time to add them. And finally, in the near future qiskit might have more than one stable supported at the same time.

Ooook I see, thanks for the release plan PR I'll read that.

But I dont have a strong opinion, just my two cents :)

Your two cents are very good to better understand where Qiskit goes and how can we follow with the Ecosystem and external projects (at least for me) 😃

Let's wait @frankharkins opinion to see where we gonna go. I think we already have some tracks we can easily apply.

@frankharkins
Copy link
Member

(Sorry, I was away and took a while to catch up.)

I vote to make stable and dev tests optional. Updating every few months is a lot of work with qiskit changing so quickly. These checks should be FYI to the maintainer. I'm still looking at the requirements problem to form a better opinion.

@mickahell
Copy link
Collaborator Author

(Sorry, I was away and took a while to catch up.)

I vote to make stable and dev tests optional. Updating every few months is a lot of work with qiskit changing so quickly. These checks should be FYI to the maintainer. I'm still looking at the requirements problem to form a better opinion.

Ok ok, I'll make the evolved in the CI

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request help wanted Extra attention is needed question Further information is requested
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants