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

Support py>=39,<py313 #108

Merged
merged 69 commits into from
Oct 31, 2024

Conversation

skupriienko
Copy link
Collaborator

@skupriienko skupriienko commented Oct 21, 2024

Actions:

  • Add the GitHub Action Workflow to support py39-py312.

    • Add the pre-commit job
    • Add the test {py39-py312} on {linux, osx, win} job (without running pytest now, see the Notes below)
  • Add dev tools:

    • Add pre-commit-config.yaml with linters and formatters
    • Add Makefile to make development easier
    • Add environment.yaml for the main environment
    • Add environment-dev.yaml for the development environment
  • Fix the build_url function by moving if resource_id: to the top of the function. It was investigated that:
    MAILJET_API_URL/contact/123456/managemanycontacts seems to be the correct order. It should also be like this https://api.mailjet.com/v3/REST/contact/CONTACT_ID/managecontactslists.
    So the Corrected url building bug #86 could be wrong (see Corrected url building bug #86 (comment)). If you replace the resource_id and action_id (e.g., MAILJET_API_URL/contact/managemanycontacts/123456), the endpoint will be unavailable and, accordingly, the test will also fail with status code 400.
    The API docs also have the correct order https://dev.mailjet.com/email/reference/contacts/subscriptions/#v3_post_contact_contact_ID_managecontactslists. Therefore, it is surprising what kind of a bug the user had 1 year ago. I think the user used the wrong mailjet.contact_managemanycontacts.get(jobID) method, although it should have been mailjet.contact_managemanycontacts.create(jobID), because the POST method always goes with create()

  • I've tested it with curl:

curl -s \
        -X POST \
        --user "$MJ_APIKEY_PUBLIC:$MJ_APIKEY_PRIVATE" \
        https://api.mailjet.com/v3/REST/contact/$CONTACT_ID/managecontactslists \
        -H 'Content-Type: application/json' \
        -d '{
      "ContactsLists":[
        {
          "Action":"addforce",
          "ListID":"$ListID"
        },
      ]
        }'

{ "Count" : 1, "Data" : [{ "ContactsLists" : [{ "Action" : "addforce", "ListID" : XXXXXX }] }], "Total" : 1 }
  • It fixes test.py::TestSuite::test_get_with_action on my local machine.

  • Packaging:

    • Replace setup.py with pyprpoject.toml
    • Remove requirements.txt as obsolete
  • Minor fixes:

    • Fix spelling in CHANGELOG.md, README.md
    • Add an empty line at the end of the LICENSE file

Notes:

@skupriienko skupriienko marked this pull request as ready for review October 24, 2024 09:57
@skupriienko skupriienko reopened this Oct 30, 2024
@skupriienko skupriienko force-pushed the DE-1055-all-python3-versions branch 2 times, most recently from 222fdec to ec90e4b Compare October 30, 2024 07:55
@skupriienko skupriienko merged commit 5872369 into mailjet:master Oct 31, 2024
39 checks passed
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.

1 participant