Skip to content

Conversation

@pierrejeambrun
Copy link
Member

@pierrejeambrun pierrejeambrun commented Sep 2, 2024

Small PR that adds sample tests for views of the new UI API.

@pierrejeambrun pierrejeambrun added the AIP-84 Modern Rest API label Sep 2, 2024
@pierrejeambrun pierrejeambrun self-assigned this Sep 2, 2024
@pierrejeambrun pierrejeambrun marked this pull request as draft September 2, 2024 14:33
@pierrejeambrun
Copy link
Member Author

I need to add the CI integration. (i.e extends the API test suite)

@pierrejeambrun pierrejeambrun marked this pull request as ready for review September 2, 2024 15:30
@bugraoz93
Copy link
Contributor

@bbovenzi @pierrejeambrun @potiuk What will be our strategy for migrating the API endpoints to the new UI API? Are we planning to do a lift and shift or are we planning to change some parts while moving them? I think we can start migrating API endpoints after this PR. Please correct me if I am wrong, we don't need to wait for follow-up work items listed here #41798 (comment). We can test the endpoints locally in parallel with those works. I wanted to align with you before starting work on migrating endpoints to the UI API.

@potiuk
Copy link
Member

potiuk commented Sep 2, 2024

What will be our strategy for migrating the API endpoints

I think it's a good topic to discuss at the next dev call (this Thursday)

@pierrejeambrun
Copy link
Member Author

pierrejeambrun commented Sep 3, 2024

bugraoz93 I would say that the initial idea is to just migrate existing endpoints to the new API (FastAPI), modifying only endpoints if necessary so we can re-wire the old UI on new endpoints and be sure that things are working as expected. Also this will minimize the migration effort and limit code duplication because at this point we can just delete the legacy endpoint.

Then we can take airflow 3 opportunity to bring any breaking change that we need for improvements / refactoring etc.

I am not sur we are fully ready to start the migration yet. Very close for 'private' endpoints i.e /objects but for the public one I will still need to figure out the auto-generation part of front end code. (types).

I would say that we can start migrating once:

  • Permissions are handled in the new API so we can also move the existing permission set to the new UI API
  • UI is re-wired on this new API (so we can safely delete the old endpoint and minimize code duplication)
  • front-end codegen works

You can keep following PRs with AIP-84 tag to stay updated, that should come in the next few days.

I think it's a good topic to discuss at the next dev call (this Thursday)

I'll write the documentation down, If we are happy with it and agree with the general strategy I can present it to the rest of the community. (This thurday if I manage to do it before then or at the next call)

@potiuk
Copy link
Member

potiuk commented Sep 3, 2024

I'll write the documentation down, If we are happy with it and agree with the general strategy I can present it to the rest of the community. (This thurday if I manage to do it before then or at the next call)

Fantastic !

@potiuk
Copy link
Member

potiuk commented Sep 3, 2024

And even if we have no write-up - we can just intiate the discussion and outline the idea this Thursday

@pierrejeambrun pierrejeambrun merged commit f84d16e into apache:main Sep 3, 2024
@pierrejeambrun pierrejeambrun deleted the ui-api-add-view-tests branch September 3, 2024 08:27
@bugraoz93
Copy link
Contributor

@pierrejeambrun Awesome! Rewiring is a great idea to reduce duplication and minimize effort. I will follow up on PRs. Thanks for the details!

@pierrejeambrun
Copy link
Member Author

pierrejeambrun commented Sep 4, 2024

Well there's been follow up on that re-wiring thing, we discussed with Brent and we will most likely keep the old endpoints with the old UI working for airflow 3 development, without re-wiring there because we want to modify/break some endpoints and that would require effort on updating the old UI (that will be deleted soon anyway).

So the idea will be to follow the same strategy that we have for front-end views. Duplicate, limit contributions on the old RestAPI and make the new UI Rest API rich enough so the old one can then be deleted.

Brent already wrote something here #41903 if you want more details.

We will suggest that on the airflow 3 community dev call this thursday and see what others think about that.

@bbovenzi
Copy link
Contributor

bbovenzi commented Sep 4, 2024

@bugraoz93 I just started a thread in the dev list too. Trying to get the old UI to use the new rest API will create a lot of work for us that we will just have to delete soon, especially if we want to redesign some endpoints. So I think the best course of action is to have duplicate code for a short time.

@bugraoz93
Copy link
Contributor

@bbovenzi @pierrejeambrun Amazing, thanks both! Yeah, I agree with both of you, if we are going to make changes then it would be hard to keep up. We will benefit from defining these things, at least from a higher level.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

AIP-84 Modern Rest API

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants