Skip to content

Improve/client: update public api and refactoring - #15

Closed
gagantrivedi wants to merge 5 commits into
mainfrom
improve/client
Closed

Improve/client: update public api and refactoring#15
gagantrivedi wants to merge 5 commits into
mainfrom
improve/client

Conversation

@gagantrivedi

Copy link
Copy Markdown
Member

Update argument name used for environment key to environment_api_key
from environment_id
Add type hints
move generate_header content to utils
Add http_headers to analytics processor which accepts a dict of headers required to talk
to the server including auth or any custom headers

Update argument name used for environment key to environment_api_key
from environment_id
Add http_headers: which accepts a dict of headers required to talk
to the server including auth or any custom headers

@dabeeeenster dabeeeenster left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

We should make this version 3.0.

Comment thread tests/test_flagsmith.py

def setUp(self) -> None:
self.bt = Flagsmith(environment_id=self.test_environment_key, api=TEST_API_URL)
self.bt = Flagsmith(environment_key=self.test_environment_key, api=TEST_API_URL)

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

We should also change this from self.bt

Comment thread flagsmith/utils.py
"""
headers = headers or {}

headers["X-Environment-Key"] = environment_key

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

We should probably pull this out into a constant

API_TOKEN_HEADER = "X-Environment-Key"

Comment thread flagsmith/flagsmith.py

def get_flags_for_user(self, identity):
def get_flags_for_user(
self, identity: str

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Since we're changing the method signatures, we should change this from identity to identifier.

@matthewelwell

matthewelwell commented Jan 27, 2022

Copy link
Copy Markdown
Contributor

I think this can be closed in favour of #17 @gagantrivedi ?

@gagantrivedi

Copy link
Copy Markdown
Member Author

Closing in favor of #17

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.

3 participants