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’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Refresh the development enviroment, care about missed "verify_ssl_cert=True" and stabilize/fix doctests in "https.txt" #397

Merged
merged 3 commits into from
Mar 25, 2021
Merged

Refresh the development enviroment, care about missed "verify_ssl_cert=True" and stabilize/fix doctests in "https.txt" #397

merged 3 commits into from
Mar 25, 2021

Conversation

amotl
Copy link
Member

@amotl amotl commented Mar 15, 2021 •
edited
Loading

Hi there,

when using Python 3.9, setting up the development environment croaked. Because bootstrap.py was a workaround in pre-pip times [1], and to modernize this repository, this patch completely gets rid of it. On the other hand, it pins to appropriate package versions to still be compatible with Python 3.5 through Python 3.9, where the former setup didn't satisfy these requirements anymore.

Along the lines, the patch fixes the doctests in https.txt in order to improve the situation with #375. For doing this appropriately, it takes care about a missed verify_ssl_cert=True setting on the HTTP client as a followup to #385.

We need this to be able to run #400 successfully on CI.

With kind regards,
Andreas.

[1] https://community.plone.org/t/not-using-bootstrap-py-as-default/620

@amotl amotl force-pushed the amo/bump-dev-versions branch 8 times, most recently from b353753 to fbdfa0a Compare March 15, 2021 18:34
@amotl amotl mentioned this pull request Mar 16, 2021
Closed
@amotl amotl changed the title Refresh the development enviroment Refresh the development enviroment, improve test layers and care about missed "verify_ssl_cert=True" Mar 16, 2021
@amotl amotl force-pushed the amo/bump-dev-versions branch 2 times, most recently from c602082 to 4b6a70b Compare March 16, 2021 12:03
@amotl amotl mentioned this pull request Mar 16, 2021
Closed
@amotl amotl requested review from mfussenegger and seut March 17, 2021 18:28
@amotl amotl marked this pull request as ready for review March 17, 2021 18:29
@amotl amotl requested a review from a team March 17, 2021 18:29
@crate crate deleted a comment from lgtm-com bot Mar 17, 2021
@mfussenegger
Copy link
Member

It's a bit hard to review as this PR does several things. Any chance to break this up into multiple smaller ones?

@amotl
Copy link
Member Author

amotl commented Mar 22, 2021

It's a bit hard to review as this PR does several things. Any chance to break this up into multiple smaller ones?

I can do it. But I will probably not be able to make CI succeed on individual chunks because currently, it is broken due to both the environment being outdated because of issues with buildout and the nasty https.txt woes on the other side of the story. However, I can try to break out 06067a2, 4eeff6f, a63e702 and d2fb2b5 into a different patch, or, on the other hand, hand in 1081140 separately.

@amotl amotl force-pushed the amo/bump-dev-versions branch 2 times, most recently from fa670d0 to 700157d Compare March 24, 2021 17:08
@amotl amotl changed the title Refresh the development enviroment, improve test layers and care about missed "verify_ssl_cert=True" Refresh the development enviroment, care about missed "verify_ssl_cert=True" and stabilize/fix doctests in "https.txt" Mar 24, 2021
@amotl
Copy link
Member Author

amotl commented Mar 24, 2021 •
edited
Loading

Hi @mfussenegger and @seut,

I removed some of the commits here in order to boil the patch down to the minimum required things to make CI succeed again. The other improvements have been submitted on behalf of #401 and #402.

With kind regards,
Andreas.

seut
seut approved these changes Mar 25, 2021
View reviewed changes
Copy link
Member

@seut seut left a comment

Choose a reason for hiding this comment

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

Looks good otherwise.

DEVELOP.rst Outdated
python3 -m venv .venv
source .venv/bin/activate

# Workaround for Python 3.9
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
# Workaround for Python 3.9
# Workaround for Python 3.5

Copy link
Member Author

Choose a reason for hiding this comment

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

Thanks for spotting this.

@seut seut mentioned this pull request Mar 25, 2021
Merged
@amotl
Refresh the development enviroment
2e5f0ab 
When using Python 3.9, setting up the development environment croaked.
Because "bootstrap.py" was a workaround in pre-pip times, this patch
completely gets rid of it.

At the same time, it modernizes some more dependency definitions and
improves the developer guide.
@amotl
Use "verify_ssl_cert=True" as a default on the CrateDB HTTP client
4f0b3a7 
Recently, this has been made a default on the CrateDB connection
object, but apparently it has been missed to also flip the switch on
this end.

Also, adjust the respective doctests accordingly.
@amotl
Improve HTTPS connection tests
9d309e1
@amotl amotl mentioned this pull request Mar 25, 2021
Open
@amotl amotl merged commit 71fb4d8 into master Mar 25, 2021
@amotl amotl deleted the amo/bump-dev-versions branch March 25, 2021 16:10
@c0c0n3 c0c0n3 mentioned this pull request Dec 15, 2021
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@mfussenegger mfussenegger mfussenegger approved these changes

@seut seut seut approved these changes

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

Successfully merging this pull request may close these issues.
3 participants
@amotl @mfussenegger @seut