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

[Feature] Finish Presto open-source client #239

Closed
1 of 13 tasks
wild-endeavor opened this issue Apr 2, 2020 · 4 comments
Closed
1 of 13 tasks

[Feature] Finish Presto open-source client #239

wild-endeavor opened this issue Apr 2, 2020 · 4 comments
Labels
enhancement New feature or request good first issue Good for newcomers stale untriaged This issues has not yet been looked at by the Maintainers

Comments

@wild-endeavor
Copy link
Contributor

wild-endeavor commented Apr 2, 2020

Motivation: Why do you think this is important?
Internally a Presto client has already been developed and is in use at Lyft (because the Presto platform at Lyft has a unique API) and the open-source Presto client remains incomplete. We should finish it.

Goal: What should the final outcome look like, ideally?
A open-source Presto client in place of the no-op client in the code today, that works with something like an Amazon Athena.

Describe alternatives you've considered
None.

Flyte component

  • Overall
  • Flyte Setup and Installation scripts
  • Flyte Documentation
  • Flyte communication (slack/email etc)
  • FlytePropeller
  • FlyteIDL (Flyte specification language)
  • Flytekit (Python SDK)
  • FlyteAdmin (Control Plane service)
  • FlytePlugins
  • DataCatalog
  • FlyteStdlib (common libraries)
  • FlyteConsole (UI)
  • Other

Additional context
Original Presto task issue: #203

@wild-endeavor wild-endeavor added enhancement New feature or request untriaged This issues has not yet been looked at by the Maintainers labels Apr 2, 2020
@wild-endeavor wild-endeavor changed the title [Feature] Finish Presto executor [Feature] Finish Presto open-source client Apr 2, 2020
@wild-endeavor
Copy link
Contributor Author

cc @lu4nm3 @kumare3

@kumare3 kumare3 added the good first issue Good for newcomers label Jul 6, 2020
eapolinario pushed a commit to eapolinario/flyte that referenced this issue Dec 6, 2022
eapolinario pushed a commit to eapolinario/flyte that referenced this issue Dec 20, 2022
* add parallelism tracking in k8s array map task

Signed-off-by: Daniel Rammer <[email protected]>

* fixed lint issues

Signed-off-by: Daniel Rammer <[email protected]>

* added unit tests

Signed-off-by: Daniel Rammer <[email protected]>

* fixed lint issues

Signed-off-by: Daniel Rammer <[email protected]>
eapolinario pushed a commit to eapolinario/flyte that referenced this issue Dec 20, 2022
* Update config_flyte_deploy.py

* update config-flyte-deploy (flyteorg#232)

Signed-off-by: Samhita Alla <[email protected]>

* Fix links & add flytectl references (flyteorg#239)

* update config-flyte-deploy

Signed-off-by: Samhita Alla <[email protected]>

* fix links

Signed-off-by: Samhita Alla <[email protected]>

Co-authored-by: Samhita Alla <[email protected]>
eapolinario pushed a commit to eapolinario/flyte that referenced this issue Dec 20, 2022
eapolinario pushed a commit to eapolinario/flyte that referenced this issue Dec 20, 2022
* Update documentation

Signed-off-by: Flyte-Bot <[email protected]>

* update

Signed-off-by: Samhita Alla <[email protected]>

* make gendocs

Signed-off-by: Samhita Alla <[email protected]>

Co-authored-by: evalsocket <[email protected]>
Co-authored-by: Samhita Alla <[email protected]>
eapolinario pushed a commit to eapolinario/flyte that referenced this issue Dec 20, 2022
* Added support for pyflyte serialize fast register (flyteorg#239)
Signed-off-by: Yuvraj <[email protected]>
eapolinario pushed a commit to eapolinario/flyte that referenced this issue Aug 9, 2023
eapolinario pushed a commit to eapolinario/flyte that referenced this issue Aug 21, 2023
* add parallelism tracking in k8s array map task

Signed-off-by: Daniel Rammer <[email protected]>

* fixed lint issues

Signed-off-by: Daniel Rammer <[email protected]>

* added unit tests

Signed-off-by: Daniel Rammer <[email protected]>

* fixed lint issues

Signed-off-by: Daniel Rammer <[email protected]>
@github-actions
Copy link

Hello 👋, This issue has been inactive for over 9 months. To help maintain a clean and focused backlog, we'll be marking this issue as stale and will close the issue if we detect no activity in the next 7 days. Thank you for your contribution and understanding! 🙏

@github-actions github-actions bot added the stale label Aug 26, 2023
@github-actions
Copy link

github-actions bot commented Sep 2, 2023

Hello 👋, This issue has been inactive for over 9 months and hasn't received any updates since it was marked as stale. We'll be closing this issue for now, but if you believe this issue is still relevant, please feel free to reopen it. Thank you for your contribution and understanding! 🙏

@github-actions github-actions bot closed this as not planned Won't fix, can't repro, duplicate, stale Sep 2, 2023
@eapolinario eapolinario reopened this Nov 2, 2023
@kumare3
Copy link
Contributor

kumare3 commented Dec 22, 2023

again defer to agent

@kumare3 kumare3 closed this as completed Dec 22, 2023
eapolinario pushed a commit to eapolinario/flyte that referenced this issue Apr 30, 2024
* Update documentation

Signed-off-by: Flyte-Bot <[email protected]>

* update

Signed-off-by: Samhita Alla <[email protected]>

* make gendocs

Signed-off-by: Samhita Alla <[email protected]>

Co-authored-by: evalsocket <[email protected]>
Co-authored-by: Samhita Alla <[email protected]>
eapolinario pushed a commit to eapolinario/flyte that referenced this issue Apr 30, 2024
* Added support for pyflyte serialize fast register (flyteorg#239)
Signed-off-by: Yuvraj <[email protected]>
austin362667 pushed a commit to austin362667/flyte that referenced this issue May 7, 2024
* Update documentation

Signed-off-by: Flyte-Bot <[email protected]>

* update

Signed-off-by: Samhita Alla <[email protected]>

* make gendocs

Signed-off-by: Samhita Alla <[email protected]>

Co-authored-by: evalsocket <[email protected]>
Co-authored-by: Samhita Alla <[email protected]>
austin362667 pushed a commit to austin362667/flyte that referenced this issue May 7, 2024
* Added support for pyflyte serialize fast register (flyteorg#239)
Signed-off-by: Yuvraj <[email protected]>
robert-ulbrich-mercedes-benz pushed a commit to robert-ulbrich-mercedes-benz/flyte that referenced this issue Jul 2, 2024
* Update documentation

Signed-off-by: Flyte-Bot <[email protected]>

* update

Signed-off-by: Samhita Alla <[email protected]>

* make gendocs

Signed-off-by: Samhita Alla <[email protected]>

Co-authored-by: evalsocket <[email protected]>
Co-authored-by: Samhita Alla <[email protected]>
robert-ulbrich-mercedes-benz pushed a commit to robert-ulbrich-mercedes-benz/flyte that referenced this issue Jul 2, 2024
* Added support for pyflyte serialize fast register (flyteorg#239)
Signed-off-by: Yuvraj <[email protected]>
troychiu pushed a commit that referenced this issue Jul 8, 2024
* Ensure token is refreshed on Unauthenticated

Signed-off-by: Haytham Abuelfutuh <[email protected]>

* Cleanup

Signed-off-by: Haytham Abuelfutuh <[email protected]>

* Add lock/unlock in BaseTokenOrchestrator as well

Signed-off-by: Haytham Abuelfutuh <[email protected]>

* generate

Signed-off-by: Haytham Abuelfutuh <[email protected]>

* Unit tests

Signed-off-by: Haytham Abuelfutuh <[email protected]>

* purge the token cache only once

Signed-off-by: Haytham Abuelfutuh <[email protected]>

* revert

Signed-off-by: Haytham Abuelfutuh <[email protected]>

* Update Purge to PurgeIfEquals

Signed-off-by: Haytham Abuelfutuh <[email protected]>

* Use cached token if one exists

Signed-off-by: Haytham Abuelfutuh <[email protected]>

* lock once

Signed-off-by: Haytham Abuelfutuh <[email protected]>

* Implement Condition Variable to avoid serializing auth requests

Signed-off-by: Haytham Abuelfutuh <[email protected]>

* Adding typed error and upgrading stow

* wip

Signed-off-by: Haytham Abuelfutuh <[email protected]>

* updating go.sum

* updating go.sum for all services with latest stow version

* fix some tests

* fix unit tests

* fix stow unit tests and linter fixes

* merge conflict fixes

* nit

---------

Signed-off-by: Haytham Abuelfutuh <[email protected]>
Co-authored-by: pmahindrakar-oss <[email protected]>
troychiu pushed a commit that referenced this issue Jul 8, 2024
This reverts commit 67e916a.

## Overview
*TODO: Provide a brief description of the feature or issue, and then summarize the changes in this PR. For larger changes make sure to callout high level design decisions. Reference relevant [design docs](https://unionai.atlassian.net/wiki/spaces/ENG/pages/78512174/Design+Review) or tracking issues.*

## Test Plan
*TODO: Summarize tests added, integration tests run, or other steps you took to validate this change. Include (or link to) relevant test output or screenshots.*

## Rollout Plan (if applicable)
*TODO: Describe any deployment or compatibility considerations for rolling out this change.*

## Upstream Changes
Should this change be upstreamed to OSS (flyteorg/flyte)? If so, please check this box for auditing. Note, this is the responsibility of each developer. See [this guide](https://unionai.atlassian.net/wiki/spaces/ENG/pages/447610883/Flyte+-+Union+Cloud+Development+Runbook/#When-are-versions-updated%3F).
- [ ] To be upstreamed

## Issue
*TODO: Link Linear issue(s) using [magic words](https://linear.app/docs/github#magic-words). `fixes` will move to merged status, while `ref` will only link the PR.*

## Checklist
* [ ] Added tests
* [ ] Ran a deploy dry run and shared the terraform plan
* [ ] Added logging and metrics
* [ ] Updated [dashboards](https://unionai.grafana.net/dashboards) and [alerts](https://unionai.grafana.net/alerting/list)
* [ ] Updated documentation
troychiu pushed a commit that referenced this issue Jul 8, 2024
#294)

…(#292)"

This reverts commit ad19bf1.

## Overview
Fixes the racy behavior in auth token cache using mutexes and condition variables.

In a nutshell, the change does the following.

* Allows all parallel token requesters to try to acquire a lock
* One of the requester gets the lock and rest get waitlisted and wait for notification from the requester who acquired the lock
* The requester who acquired the lock refreshes the token and save to the token cache
* He also notifies the waitlisted requesters to now go and read the saved token cache and proceed with the request

The test plan below summarizes how this was tested

## Test Plan

https://buildkite.com/unionai/org-staging-sync/builds/2311
https://buildkite.com/unionai/managed-cluster-staging-sync/builds/3461


Serverless onboarding workshop 
https://buildkite.com/unionai/serverless-workshop-staging-deploy-and-run/builds/44#018fa797-269b-4c08-9809-f6561f8af777

This was tested locally aswell using integration test and was ran using 20 parallel routines
```
go test -v -tags=integration -run '^TestAPISuite$/^Test_RandomNameWorkflow$'
```

* All routines get unauthenticated in the first attempt of using inmemory cached token provider
```
cat t |grep "If it's an unauthenticated error, we will attempt to establish an authenticated context" |wc -l
      20
```
*  All routines try to lock
```
cat t |grep "try lock"  |wc -l                                                         python
      20
```
*  One of the routine is able to acquire the lock
```
cat t |grep "Locked : true"  |wc -l                                                    python
       1
```

* Rest fail to lock
```
cat t |grep "Locked : false"  |wc -l                                                   python
      19
```
* The ones who fail to lock go into waiting state . i.e they get added to condition variables waitlist

```
cat t |grep "Waiting" |grep -v "Coming" |wc -l                                         python
      19
```

eg : log for ref
```
{"json":{"src":"token_cache_inmemory.go:77"},"level":"info","msg":"Coming out of Waiting","ts":"2024-05-22T23:52:47-07:00"}
```

* The one who has locked goes and saves the token
```
cat t |grep "Saved"  |wc -l                                                            python
       1
```

* Broadcasts to all the others waiting using single notification which notifies all the waiters.
```
cat t |grep "Broadcasted"  |wc -l                                                      python
       1
```

* Post which they come out of waiting
```
 cat t |grep "Coming out of Waiting"|wc -l                                              python
      19
```

And go ahead and read the token


Log file for reference

[t.txt](https://github.com/unionai/flyte/files/15413364/t.txt)

## Rollout Plan (if applicable)
Merge this change and merge the cloud PR 

## Upstream Changes
Should this change be upstreamed to OSS (flyteorg/flyte)? If so, please check this box for auditing. Note, this is the responsibility of each developer. See [this guide](https://unionai.atlassian.net/wiki/spaces/ENG/pages/447610883/Flyte+-+Union+Cloud+Development+Runbook/#When-are-versions-updated%3F).

- [X] To be upstreamed

## Issue
*TODO: Link Linear issue(s) using [magic words](https://linear.app/docs/github#magic-words). `fixes` will move to merged status, while `ref` will only link the PR.*

## Checklist
* [ ] Added tests
* [ ] Ran a deploy dry run and shared the terraform plan
* [ ] Added logging and metrics
* [ ] Updated [dashboards](https://unionai.grafana.net/dashboards) and [alerts](https://unionai.grafana.net/alerting/list)
* [ ] Updated documentation
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request good first issue Good for newcomers stale untriaged This issues has not yet been looked at by the Maintainers
Projects
None yet
Development

No branches or pull requests

3 participants