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

[BUG] Clean up flytekit models #414

Open
1 of 20 tasks
wild-endeavor opened this issue Jul 14, 2020 · 4 comments
Open
1 of 20 tasks

[BUG] Clean up flytekit models #414

wild-endeavor opened this issue Jul 14, 2020 · 4 comments
Assignees
@eapolinario
Labels
bug Something isn't working flytekit FlyteKit Python related issue good first issue Good for newcomers stale

Comments

@wild-endeavor
Copy link
Contributor

Describe the bug
This isn't really a bug but it's also not really a feature. Flytekit currently has a series of model files that are supposed to mimic the IDL structure.

For example this ConnectionSet class is supposed to mirror this protobuf message. Every single IDL object that's used in flytekit is mirrored in this way because the generated Python code doesn't play nicely with autocomplete and IDEs and such. It's just much nicer to work with these Python classes. In the future, we should also investigate writing a proto compiler to autogenerate these.

In the meantime, there are a few inconsistencies that we've accidentally written

  • There are two CompiledTasks. We should remove one of them (it's not trivial, i think it causes a cyclic import somewhere).
  • Some of the objects like this are Admin objects, but they're not in the admin folder. We should probably move these into the admin folder. The admin folder for flytekit models should mirror the admin folder in the IDL. If not, we should at least have very good comments explaining why not.
  • Some of the type hints are also incorrect. We should fix these.

Expected behavior
Please see above. We should be careful not to break anything.

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

To Reproduce
Steps to reproduce the behavior:
NA

Screenshots
NA

Environment
Flyte component

  • Sandbox (local or on one machine)
  • Cloud hosted
    • AWS
    • GCP
    • Azure
  • Baremetal
  • Other

Additional context
NA
@wild-endeavor wild-endeavor added bug Something isn't working untriaged This issues has not yet been looked at by the Maintainers labels Jul 14, 2020
@wild-endeavor wild-endeavor changed the title [BUG] [BUG] Clean up flytekit models Jul 14, 2020
@wild-endeavor wild-endeavor added the good first issue Good for newcomers label Jul 14, 2020
@EngHabu EngHabu added flytekit FlyteKit Python related issue bugSquash-Cascade labels Aug 30, 2021
@EngHabu EngHabu added this to the 0.18.0 milestone Aug 31, 2021
@wild-endeavor wild-endeavor removed this from the 0.18.0 milestone Sep 1, 2021
@mayitbeegh mayitbeegh self-assigned this Sep 1, 2021
eapolinario pushed a commit to eapolinario/flyte that referenced this issue Dec 6, 2022
@hamersaw
Create FlyteWorkflow CRD on FlytePropeller startup (flyteorg#414)
46566e6 
* creating FlyteWorkflow CRD on FlytePropeller startup

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

* checking for CRD existence before creating

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

* added CreateFlyteWorkflowCRD configuration option

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

* fixed CreateFlyteWorkflowCRD configuration description

Signed-off-by: Daniel Rammer <[email protected]>
eapolinario pushed a commit to eapolinario/flyte that referenced this issue Dec 6, 2022
@katrogan
Update github tag action (flyteorg#414)
aa53a53 
Signed-off-by: Katrina Rogan <[email protected]>
eapolinario pushed a commit to eapolinario/flyte that referenced this issue Dec 20, 2022
@yindia @katrogan @samhita-alla
Added example for horovod mpi (flyteorg#414)
69ef957 
* Added example for MPI

Signed-off-by: Yuvraj <[email protected]>

Co-authored-by: Katrina Rogan <[email protected]>
Co-authored-by: Samhita Alla <[email protected]>
eapolinario pushed a commit to eapolinario/flyte that referenced this issue Aug 9, 2023
@hamersaw
Create FlyteWorkflow CRD on FlytePropeller startup (flyteorg#414)
15d2eab 
* creating FlyteWorkflow CRD on FlytePropeller startup

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

* checking for CRD existence before creating

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

* added CreateFlyteWorkflowCRD configuration option

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

* fixed CreateFlyteWorkflowCRD configuration description

Signed-off-by: Daniel Rammer <[email protected]>
eapolinario pushed a commit to eapolinario/flyte that referenced this issue Aug 21, 2023
@katrogan
Update github tag action (flyteorg#414)
32c51c1 
Signed-off-by: Katrina Rogan <[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 pushed a commit that referenced this issue Sep 8, 2023
@pingsutw
Add tags to execution spec (#414)
3e852b0 
* add tags to execution spec

Signed-off-by: Kevin Su <[email protected]>

* add tags to execution spec

Signed-off-by: Kevin Su <[email protected]>

* add comment

Signed-off-by: Kevin Su <[email protected]>

---------

Signed-off-by: Kevin Su <[email protected]>
eapolinario pushed a commit that referenced this issue Sep 13, 2023
@pingsutw @eapolinario
Add tags to execution spec (#414)
2871a17 
* add tags to execution spec

Signed-off-by: Kevin Su <[email protected]>

* add tags to execution spec

Signed-off-by: Kevin Su <[email protected]>

* add comment

Signed-off-by: Kevin Su <[email protected]>

---------

Signed-off-by: Kevin Su <[email protected]>
Signed-off-by: Eduardo Apolinario <[email protected]>
eapolinario pushed a commit that referenced this issue Sep 26, 2023
@wild-endeavor @honnix
@pingsutw @katrogan @jeevb @hamersaw
GPU Type (#419)
7bd98a9 
* add field

Signed-off-by: Yee Hing Tong <[email protected]>
Signed-off-by: Jeev B <[email protected]>

* Pass task execution metadata from agent (#422)

* Pass task execution metadata from agent

Signed-off-by: Hongxin Liang <[email protected]>

* Add doc

Signed-off-by: Hongxin Liang <[email protected]>

* Update protos/flyteidl/admin/agent.proto

Co-authored-by: Kevin Su <[email protected]>
Signed-off-by: Honnix <[email protected]>

* Regenerate

---------

Signed-off-by: Hongxin Liang <[email protected]>
Signed-off-by: Honnix <[email protected]>
Co-authored-by: Kevin Su <[email protected]>
Signed-off-by: Jeev B <[email protected]>

* Add tags to execution spec (#414)

* add tags to execution spec

Signed-off-by: Kevin Su <[email protected]>

* add tags to execution spec

Signed-off-by: Kevin Su <[email protected]>

* add comment

Signed-off-by: Kevin Su <[email protected]>

---------

Signed-off-by: Kevin Su <[email protected]>
Signed-off-by: Jeev B <[email protected]>

* Correct comment for array job max parallelism (#431)

Signed-off-by: Katrina Rogan <[email protected]>
Signed-off-by: Jeev B <[email protected]>

* Add the scalar to the operand (#427)

Signed-off-by: Kevin Su <[email protected]>
Signed-off-by: Jeev B <[email protected]>

* add selector

Signed-off-by: Yee Hing Tong <[email protected]>
Signed-off-by: Jeev B <[email protected]>

* move selectors from container to task metadata

Signed-off-by: Yee Hing Tong <[email protected]>
Signed-off-by: Jeev B <[email protected]>

* drop only_preferred

Signed-off-by: Jeev B <[email protected]>

* Updating boilerplate to lock golangci-lint version (#435)

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

* add unpartitioned selector

Signed-off-by: Jeev B <[email protected]>

* refactor

Signed-off-by: Jeev B <[email protected]>

* refactor

Signed-off-by: Jeev B <[email protected]>

* fix oneof names

Signed-off-by: Jeev B <[email protected]>

* add build.os for read the docs

Signed-off-by: Jeev B <[email protected]>

---------

Signed-off-by: Yee Hing Tong <[email protected]>
Signed-off-by: Jeev B <[email protected]>
Signed-off-by: Hongxin Liang <[email protected]>
Signed-off-by: Honnix <[email protected]>
Signed-off-by: Kevin Su <[email protected]>
Signed-off-by: Katrina Rogan <[email protected]>
Signed-off-by: Daniel Rammer <[email protected]>
Co-authored-by: Honnix <[email protected]>
Co-authored-by: Kevin Su <[email protected]>
Co-authored-by: Kevin Su <[email protected]>
Co-authored-by: Katrina Rogan <[email protected]>
Co-authored-by: Jeev B <[email protected]>
Co-authored-by: Dan Rammer <[email protected]>
@eapolinario eapolinario reopened this Nov 2, 2023
@kumare3 kumare3 removed untriaged This issues has not yet been looked at by the Maintainers stale labels Dec 22, 2023
@kumare3
Copy link
Contributor

kumare3 commented Dec 22, 2023

i have heard there is a surprise coming

pvditt pushed a commit that referenced this issue Dec 29, 2023
@pingsutw
Add tags to execution spec (#414)
305556c 
* add tags to execution spec

Signed-off-by: Kevin Su <[email protected]>

* add tags to execution spec

Signed-off-by: Kevin Su <[email protected]>

* add comment

Signed-off-by: Kevin Su <[email protected]>

---------

Signed-off-by: Kevin Su <[email protected]>
pvditt pushed a commit that referenced this issue Dec 29, 2023
@wild-endeavor @honnix
@pingsutw @katrogan @jeevb @hamersaw
GPU Type (#419)
b92acad 
* add field

Signed-off-by: Yee Hing Tong <[email protected]>
Signed-off-by: Jeev B <[email protected]>

* Pass task execution metadata from agent (#422)

* Pass task execution metadata from agent

Signed-off-by: Hongxin Liang <[email protected]>

* Add doc

Signed-off-by: Hongxin Liang <[email protected]>

* Update protos/flyteidl/admin/agent.proto

Co-authored-by: Kevin Su <[email protected]>
Signed-off-by: Honnix <[email protected]>

* Regenerate

---------

Signed-off-by: Hongxin Liang <[email protected]>
Signed-off-by: Honnix <[email protected]>
Co-authored-by: Kevin Su <[email protected]>
Signed-off-by: Jeev B <[email protected]>

* Add tags to execution spec (#414)

* add tags to execution spec

Signed-off-by: Kevin Su <[email protected]>

* add tags to execution spec

Signed-off-by: Kevin Su <[email protected]>

* add comment

Signed-off-by: Kevin Su <[email protected]>

---------

Signed-off-by: Kevin Su <[email protected]>
Signed-off-by: Jeev B <[email protected]>

* Correct comment for array job max parallelism (#431)

Signed-off-by: Katrina Rogan <[email protected]>
Signed-off-by: Jeev B <[email protected]>

* Add the scalar to the operand (#427)

Signed-off-by: Kevin Su <[email protected]>
Signed-off-by: Jeev B <[email protected]>

* add selector

Signed-off-by: Yee Hing Tong <[email protected]>
Signed-off-by: Jeev B <[email protected]>

* move selectors from container to task metadata

Signed-off-by: Yee Hing Tong <[email protected]>
Signed-off-by: Jeev B <[email protected]>

* drop only_preferred

Signed-off-by: Jeev B <[email protected]>

* Updating boilerplate to lock golangci-lint version (#435)

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

* add unpartitioned selector

Signed-off-by: Jeev B <[email protected]>

* refactor

Signed-off-by: Jeev B <[email protected]>

* refactor

Signed-off-by: Jeev B <[email protected]>

* fix oneof names

Signed-off-by: Jeev B <[email protected]>

* add build.os for read the docs

Signed-off-by: Jeev B <[email protected]>

---------

Signed-off-by: Yee Hing Tong <[email protected]>
Signed-off-by: Jeev B <[email protected]>
Signed-off-by: Hongxin Liang <[email protected]>
Signed-off-by: Honnix <[email protected]>
Signed-off-by: Kevin Su <[email protected]>
Signed-off-by: Katrina Rogan <[email protected]>
Signed-off-by: Daniel Rammer <[email protected]>
Co-authored-by: Honnix <[email protected]>
Co-authored-by: Kevin Su <[email protected]>
Co-authored-by: Kevin Su <[email protected]>
Co-authored-by: Katrina Rogan <[email protected]>
Co-authored-by: Jeev B <[email protected]>
Co-authored-by: Dan Rammer <[email protected]>
eapolinario pushed a commit to eapolinario/flyte that referenced this issue Apr 30, 2024
@DavidMertz
Consistent language about 'git revision SHA1' (flyteorg#414)
d521e8b 
Signed-off-by: David Mertz <[email protected]>
eapolinario pushed a commit to eapolinario/flyte that referenced this issue Apr 30, 2024
@DavidMertz
Consistent language about 'git revision SHA1' (flyteorg#414)
ee25d67 
Signed-off-by: David Mertz <[email protected]>
austin362667 pushed a commit to austin362667/flyte that referenced this issue May 7, 2024
@DavidMertz @austin362667
Consistent language about 'git revision SHA1' (flyteorg#414)
b2de010 
Signed-off-by: David Mertz <[email protected]>
robert-ulbrich-mercedes-benz pushed a commit to robert-ulbrich-mercedes-benz/flyte that referenced this issue Jul 2, 2024
@DavidMertz @robert-ulbrich-mercedes-benz
Consistent language about 'git revision SHA1' (flyteorg#414)
e7f0180 
Signed-off-by: David Mertz <[email protected]>
@github-actions 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 engage on it to decide if it is still applicable.
Thank you for your contribution and understanding! 🙏

@github-actions github-actions bot added the stale label Sep 18, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@eapolinario eapolinario

Labels
bug Something isn't working flytekit FlyteKit Python related issue good first issue Good for newcomers stale
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
5 participants
@EngHabu @eapolinario @wild-endeavor @mayitbeegh @kumare3