[dagster-graphql-client] Allow HTTPS use
ClosedPublic
Actions

Authored by • sidkmenon on Jun 9 2021, 9:37 PM.

Details

Reviewers

• dgibson
• owen

Commits

R1:40c548297a99: [dagster-graphql-client] Allow HTTPS use

Summary

After a lot of back and forth, we were able to finally sort out this issue as related to using http vs. https.

Therefore, adding a flag that allows users to specify whether or not they want to use HTTPS in the client's connection URL.

Test Plan

Repository

R1 dagster

Lint

Lint Not Applicable

Unit

Tests Not Applicable

• sidkmenon requested review of this revision.Jun 9 2021, 10:02 PM

• dgibson added inline comments.

python_modules/dagster-graphql/dagster_graphql/client/client.py
45–51	nit Is there a reason optional is listed separately if we have the Optional type? why not use_https (Optional[bool])?

This revision is now accepted and ready to land.Jun 9 2021, 10:22 PM

• sidkmenon marked an inline comment as done.Jun 9 2021, 11:19 PM

• sidkmenon added inline comments.

python_modules/dagster-graphql/dagster_graphql/client/client.py
45–51	nit Is there a reason optional is listed separately if we have the Optional type? why not use_https (Optional[bool])? I think this is how Sphinx generates docstrings by default - on some reflection, I think this is why: `Optional[bool]` denotes `use_https : True \| False \| None`; but it does not necessarily imply that `use_https` has a default value. `bool, optional` denotes that `use_https: True \| False` but it is has a default value, so does not need to be specified