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

Make requirements granular and optional #34

Merged
merged 4 commits into from
Apr 18, 2017
Merged

Make requirements granular and optional #34

merged 4 commits into from
Apr 18, 2017

Conversation

osantana
Copy link
Contributor

@osantana osantana commented Apr 18, 2017

We can declare requirements like:

  • correios>=1.0.0 - only model classes and validators
  • correios[pdf]>=1.0.0 - base installation + label/posting-list pdf generation
  • correios[client]>=1.0.0 - base installation + sigep/websro api client
  • correios[client,pdf]>=1.0.0 - full installation

@osantana osantana added the wip label Apr 18, 2017
@lamenezes
Copy link
Contributor

We should at least add these installation options to the README.

IMHO these options should be a little different:

  • correios>=1.0.0 - full install
  • correios[model] - models
  • correios[model,api] - models + api
  • correios[api,pdf] - pdf + api

@osantana
Copy link
Contributor Author

IMHO these options should be a little different:

correios>=1.0.0 - full install

pip does not support this

@osantana
Copy link
Contributor Author

We use correios library at several services at Olist but only one of them use PDF generation. And the API client is used only in 2 services. So, the "full installation" profile (model + pdf + api) is never used.

But obviously this change breaks with their dependencies and that is why I bumped the major version.

result = client.request_tracking_codes(user, Service.get(SERVICE_SEDEX), quantity=10)
assert len(result) == 10
assert len(result[0].code) == 13


@pytest.mark.skipif(not correios, reason="API Client support disabled")
Copy link
Contributor

Choose a reason for hiding this comment

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

se esta usando o cassette por que o skip ? Tem risco de no 1 teste quando esta gravando o cassette a API esta fora ?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

quebra no import porque o módulo client não vai encontrar o suds, por exemplo...

Copy link
Contributor

@rhenter rhenter left a comment

Choose a reason for hiding this comment

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

👍

@caiocarrara
Copy link
Contributor

@adlermedrado and @lamenezes it would be awesome if you review it asap. This change and the other planned to be merged after this one are important for what we're developing.

@lamenezes lamenezes merged commit 2618bdd into master Apr 18, 2017
@lamenezes lamenezes deleted the granular-reqs branch April 18, 2017 18:47
@osantana osantana restored the granular-reqs branch April 19, 2017 14:03
@osantana osantana deleted the granular-reqs branch April 19, 2017 14:04
@osantana osantana restored the granular-reqs branch April 19, 2017 14:07
@osantana osantana deleted the granular-reqs branch April 27, 2017 21:06
osantana pushed a commit that referenced this pull request Apr 28, 2017
Make requirements granular and optional
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants