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

switch to httpx (#74) #75

Merged
merged 3 commits into from Dec 13, 2022
Merged

switch to httpx (#74) #75

merged 3 commits into from Dec 13, 2022

Conversation

FHU-yezi
Copy link
Contributor

No description provided.

@Alek99 Alek99 self-requested a review December 12, 2022 05:35
@picklelo
Copy link
Contributor

Code looks good, I'll look into the Github Actions failure... we can merge once it's passing

@Alek99 Alek99 added the enhancement Anything you want improved label Dec 12, 2022
@Alek99 Alek99 added this to the v0.1.9 milestone Dec 12, 2022
@FHU-yezi
Copy link
Contributor Author

FHU-yezi commented Dec 12, 2022

I know why the CI is failed. Poetry just released a new version of lock file, and I use the latest version. When I run poetry lock to sync the lock file with the TOML file, it automaticlly update the lock file to the latest version.

Update the poetry dependence to latest version should solve this problem.

Poetry v1.3.0 release log: https://github.com/python-poetry/poetry/releases/tag/1.3.0

The PR of lockfile change: python-poetry/poetry#6393

@FHU-yezi
Copy link
Contributor Author

I got some error when I try to push the commit to my repo:

remote: Resolving deltas: 100% (2/2), completed with 2 local objects.
To https://github.com/FHU-yezi/pynecone.git
 ! [remote rejected] httpx -> httpx (refusing to allow a Personal Access Token to create or update workflow `.github/workflows/build.yml` without `workflow` scope)

Don't know how to deal with it, my change:

# .github/workflows/build.yml, line 28
    - name: cache poetry install
      uses: actions/cache@v2
      with:
        path: ~/.local
        key: python-${{ matrix.python-version }}-poetry-1.2.2-0

    - uses: snok/install-poetry@v1
      with:
        version: 1.2.2
        virtualenvs-create: true
        virtualenvs-in-project: true

@picklelo
Copy link
Contributor

picklelo commented Dec 12, 2022

I think you may not be able to update the CI jobs - I'll push a PR updating it with the poetry version, then we can merge this.

@Alek99
Copy link
Contributor

Alek99 commented Dec 12, 2022

I know why the CI is failed. Poetry just released a new version of lock file, and I use the latest version. When I run poetry lock to sync the lock file with the TOML file, it automaticlly update the lock file to the latest version.

Thanks good catch

@picklelo
Copy link
Contributor

@FHU-yezi I just pushed a PR updating the poetry version, could you rebase this on top and push? Hopefully the build passes this time.

Btw, thanks for all your contributions and involvement with the project so far!

@picklelo picklelo linked an issue Dec 13, 2022 that may be closed by this pull request
@FHU-yezi FHU-yezi closed this Dec 13, 2022
@FHU-yezi FHU-yezi reopened this Dec 13, 2022
@FHU-yezi
Copy link
Contributor Author

Just rebased my branch with the newest commits, and now the CI is running... But it failed again.

@FHU-yezi
Copy link
Contributor Author

FHU-yezi commented Dec 13, 2022

Read httpx documentation and tried by myself, this way of file upload will still work. Maybe developers don't think someone will upload files in request body directly, so I add some type ignores.

By the way, I removed some f which is unnesserary now.

All CI show me the green light, we are about to merge.

Copy link
Contributor

@picklelo picklelo left a comment

Choose a reason for hiding this comment

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

Awesome

@picklelo picklelo merged commit 2028606 into reflex-dev:main Dec 13, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement Anything you want improved
Projects
None yet
Development

Successfully merging this pull request may close these issues.

replace requests with httpx
3 participants