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.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We鈥檒l occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Remove PostgresJobStore (compatibility layer) #188

Merged
merged 2 commits into from Apr 21, 2020
Merged

Remove PostgresJobStore (compatibility layer) #188

merged 2 commits into from Apr 21, 2020

Conversation

elemoine
Copy link
Contributor

This removes the PostgresJobStore compatibility layer.

Closes #187.

Successful PR Checklist:

  • Tests Not relevant
  • Documentation (optionally: run spell checking) Not relevant
  • Had a good time contributing? (if not, feel free to give some feedback) Not relevant 馃槃

@codecov-io
Copy link

codecov-io commented Apr 20, 2020 •
edited

Codecov Report

Merging #188 into master will decrease coverage by 0.00%.
The diff coverage is 100.00%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #188      +/-   ##
==========================================
- Coverage   99.78%   99.77%   -0.01%     
==========================================
  Files          21       21              
  Lines         912      898      -14     
  Branches       98       96       -2     
==========================================
- Hits          910      896      -14     
  Misses          1        1              
  Partials        1        1
Impacted Files Coverage 螖
procrastinate/aiopg_connector.py 97.70% <酶> (-0.15%) 猬囷笍
procrastinate/app.py 100.00% <酶> (酶)
procrastinate/__init__.py 100.00% <100.00%> (酶)

Continue to review full report at Codecov.

Legend - Click here to learn more
螖 = absolute <relative> (impact), 酶 = not affected, ? = missing data
Powered by Codecov. Last update e31df23...14bba35. Read the comment docs.

ewjoachim
ewjoachim reviewed Apr 21, 2020
View reviewed changes
Copy link
Member

@ewjoachim ewjoachim left a comment

Choose a reason for hiding this comment

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

There's also the "job_store" arg of the App

@elemoine
Copy link
Contributor Author

There's also the "job_store" arg of the App

Yep, fixed.

ewjoachim
ewjoachim reviewed Apr 21, 2020
View reviewed changes
procrastinate/app.py Outdated
)
logger.warning(f"Deprecation Warning: {message}")
warnings.warn(DeprecationWarning(message))
connector = job_store
if not connector:
Copy link
Member

Choose a reason for hiding this comment

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

Connector can now be a mandatory argument and this if branch can be removed.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yeah, I was too fast.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Fixed.

@elemoine
Copy link
Contributor Author

We're getting hit by pytest-dev/pytest-asyncio#154.

@elemoine
Copy link
Contributor Author

@ewjoachim pytest-asyncio 0.11.0 broke our tests (see pytest-dev/pytest-asyncio#154). So I added a commit to use pytest-asyncio<0.11.0 until this is fixed.

Do not use pytest-asyncio 0.11.0
14bba35 
tests/integration/test_aiopg_connector.py::test_execute_query_simultaneous fails
with pytest-asyncio 0.11.0.

See pytest-dev/pytest-asyncio#154.
ewjoachim
ewjoachim approved these changes Apr 21, 2020
View reviewed changes
Copy link
Member

@ewjoachim ewjoachim left a comment •
edited

Choose a reason for hiding this comment

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

Yeah, it's better with a != than a <, because this will allow us to use future version when they fix it, or be informed if they don't.

@elemoine elemoine merged commit 9dd6c29 into master Apr 21, 2020
@elemoine elemoine deleted the ele_pg-job-store branch April 21, 2020 08:53
@elemoine
Copy link
Contributor Author

elemoine commented May 4, 2020

@ewjoachim pytest-asyncio 0.11.0 broke our tests (see pytest-dev/pytest-asyncio#154). So I added a commit to use pytest-asyncio<0.11.0 until this is fixed.

Good news. The problem is fixed with pytest-asyncio 0.12.0, which just came out.

@ewjoachim
Copy link
Member

But the nice thing is that we don't even need to change anything now.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@ewjoachim ewjoachim ewjoachim approved these changes

@DainDwarf DainDwarf Awaiting requested review from DainDwarf

@mgu mgu Awaiting requested review from mgu

@pmourlanne pmourlanne Awaiting requested review from pmourlanne

@sophie-ulti sophie-ulti Awaiting requested review from sophie-ulti

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Remove the PostgresJobStore compatibility layer
3 participants
@elemoine @codecov-io @ewjoachim