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

Feat/handshake config #668

Merged
merged 31 commits into from
Oct 10, 2024
Merged

Feat/handshake config #668

merged 31 commits into from
Oct 10, 2024

Conversation

CTLalit
Copy link
Collaborator

@CTLalit CTLalit commented Sep 16, 2024

- adds manifest tag as placeholder
- parses the manifest label and saves in string
- adds constants and fluff
- base url impl was hidden
- readable code to see url building and passing
- adds custom domain through instance to api class
- modifies tests to handle new change
- will think of better design to hold account vars together
- proper request handling
- naming for viewed event endpoint check
- needs handshake is changed as per new data in manifest
- adds test case for above to return handshake url
- separates handshake and other api call url builder
- verbose url building
- adds cached handshake filed in ct api
- corrects set handshake domain method
- names to call args in CtApi.kt
- rename cached fields to make tests readable
- idiomatic kotlin way in UrlConnectionHttpClient.kt
- adds custom handshake header as needed by feature
- headers gets added only in handshake request and if mentioned in manifest
- adds tests to confirm the headers and url
Copy link
Contributor

@Anush-Shand Anush-Shand left a comment

Choose a reason for hiding this comment

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

Added comments @CTLalit

- sets data from manifest to default instance
- moves method to ct extensions
- fixes logging
- test creds removal
Copy link
Contributor

@Anush-Shand Anush-Shand left a comment

Choose a reason for hiding this comment

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

LGTM

CTLalit and others added 12 commits September 24, 2024 16:29
- needs no handshake when mentioned first time, migration to be done via redirection headers in /a1 api call
- changes methods in ctapi, also need to clear any previously cached handshake performance info (did/did not do)
- removes cached handshake info as its not needed
- there were checks based on cached info so we can identify config added first time
Network layer changes for handshake support.
- changes are made to logic so tests are changed acc
- ignores flaky test cases
- compliances around image urls we need to use need to be considered
feat(SDK-4076): Changes need handshake logic
- cleaned up the class and added easier access to create instance with config
- adds method for change creds for custom handshaking.
Copy link
Contributor

@piyush-kukadiya piyush-kukadiya left a comment

Choose a reason for hiding this comment

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

LGTM

@CTLalit CTLalit merged commit f976c6b into develop Oct 10, 2024
4 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.

4 participants