Skip to content

AIP-81 - API Communication Mechanism - #45300

Merged
jedcunningham merged 19 commits into
apache:mainfrom
bugraoz93:feat/42561/cli-central-api-communication
Mar 19, 2025
Merged

AIP-81 - API Communication Mechanism#45300
jedcunningham merged 19 commits into
apache:mainfrom
bugraoz93:feat/42561/cli-central-api-communication

Conversation

@bugraoz93

@bugraoz93bugraoz93 commented Dec 30, 2024

Copy link
Copy Markdown
Contributor

closes: #42561 and #43656

Summary
This PR introduces a central API communication mechanism for the CLI and adopts one of the broadest endpoint ranges in the API, focusing on connection commands. After several iterations to automate API call generation and handle edge cases, I couldn't achieve a fully dynamic request generation process. Instead, I landed on a structure similar to TaskSDK <-> ExecutionAPI. This approach simplifies reviews, maintains consistency across clients, and opens the door for better automation in future iterations, possibly in 3.1 or 3.2.

Implementation Details

  • Added a central API communication mechanism for the CLI.
  • Implemented most of the necessary CLI operations.
  • Data model generation from RestAPI to CLI is handled using uv.
  • Added decorators to cascade generic processes to operations.
  • Created a testing framework for CLI testing, with an example implementation for connection command.
  • Pending Work
  • Some minor features related to the connection command are still in progress and will be added in follow-up updates before 3.0.

Note: There are two missing functionalities in this PR for the connection command. Those will be included with follow-up tasks.

Notes on task_command
While working on this, I realized that task_command behaves more like a local_command than a remote_command. This needs further discussion, especially regarding its integration with TaskSDK. From the CLI perspective, depending on TaskSDK seems cleaner than directly calling ExecutionAPI, it avoids duplication and keeps the structure consistent. Even if the CLI calls TaskSDK, the two-hop process to reach the ExecutionAPI feels a bit redundant. What do you think, @ashb (to all, please jump into discussion, I tagged Ash to follow up the previous discussion started on this in Slack)?


^ Add meaningful description above
Read the Pull Request Guidelines for more information.
In case of fundamental code changes, an Airflow Improvement Proposal (AIP) is needed.
In case of a new dependency, check compliance with the ASF 3rd Party License Policy.
In case of backwards incompatible changes please leave a note in a newsfragment file, named {pr_number}.significant.rst or {issue_number}.significant.rst, in newsfragments.

@boring-cyborgboring-cyborgBot added area:CLI area:UI Related to UI/UX. For Frontend Developers. labels Dec 30, 2024
@bugraoz93bugraoz93 added the area:API Airflow's REST/HTTP API label Dec 30, 2024
Comment threadairflow/cli/api/cli_api_client.py Outdated
Comment threadairflow/cli/api/operations.py
@jscheffl

Copy link
Copy Markdown
Contributor

Note: As PR #45312 has been merged, the code formatting rules have changed for new UI. Please rebase and re-run pre-commit checks to ensure that formatting in folder airflow/ui is adjusted.

@bugraoz93
bugraoz93force-pushed the feat/42561/cli-central-api-communication branch from 8446931 to 776cf69CompareJanuary 2, 2025 17:31
@bugraoz93bugraoz93 changed the title Central API Communication Mechanism for CLI and Connection Command IntegrationAIP-81 Central API Communication Mechanism for CLI and Connection Command IntegrationJan 2, 2025
@bugraoz93
bugraoz93force-pushed the feat/42561/cli-central-api-communication branch 3 times, most recently from 449e9ab to bd4a661CompareJanuary 8, 2025 22:51
@bugraoz93

Copy link
Copy Markdown
ContributorAuthor

Note Update: The missing parts are included in the PR.

  • Create default connections in connection_command
  • Include overwrite functionality for file import operations for connections and pool endpoint
  • Include overwrite functionality to connection_command

Thanks @jason810496 for your contributions!

Comment threadairflow/cli/api/datamodels/_generated.py
@bugraoz93bugraoz93 mentioned this pull request Jan 15, 2025
2 tasks
@hazemAmr0hazemAmr0 mentioned this pull request Jan 21, 2025
2 tasks
@bugraoz93
bugraoz93force-pushed the feat/42561/cli-central-api-communication branch from bd4a661 to 1a17613CompareJanuary 23, 2025 23:20
Comment threadairflow/cli/api/__init__.py
Comment threadairflow/cli/api/client.py Outdated
Comment threadairflow/cli/api/client.py Outdated
Comment threadairflow/cli/api/client.py Outdated

@jscheffljscheffl 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.

I saw this PR and was "afraid" from the size but after the dev call today I looked again. Agree to comments from @ashb - after this I assume it can be merged.

Next time would be better to cut PRs in smaller chunks not to scare reviewers away by number of LoC :-D

@bugraoz93

Copy link
Copy Markdown
ContributorAuthor

Thanks a lot for your time and your comments! I will address them and adjust the code accordingly soon.

Next time would be better to cut PRs in smaller chunks not to scare reviewers away by number of LoC :-D

Indeed, that's on me making it big. I thought most parts were repeated on the operations while making the integration easier for contributers and went for it. I agree, that's on me, I will definitely split next time :D

… most remote commands, connection_command.py integrated with client, data model generation from ResAPI to CLI is added using uv similar to TaskSDK
…nd operations, include create default connection_command, update generated models
…g retrieving token, remove world readable file name, make environment separation and control with environment variable, control the token with both path and environment variable, remove additional configure action
…o always raise and let the caller decide, add back exceptions into connection_command, remove comments from pyproject.toml
@bugraoz93
bugraoz93force-pushed the feat/42561/cli-central-api-communication branch from 1c9e80d to cacf34aCompareMarch 19, 2025 22:11
@jedcunningham

Copy link
Copy Markdown
Member

Failure is unrelated. Merging!

@jedcunningham
jedcunningham merged commit 81893ed into apache:mainMar 19, 2025
@jedcunningham

Copy link
Copy Markdown
Member

#protm

@jason810496

Copy link
Copy Markdown
Member

Hi @bugraoz93, since this PR has been merged, I think we can start working on the #45661 meta issue, right? Or are there any other dependencies that need to be completed first?

@bugraoz93

Copy link
Copy Markdown
ContributorAuthor

Hi @bugraoz93, since this PR has been merged, I think we can start working on the #45661 meta issue, right? Or are there any other dependencies that need to be completed first?

Hi @jason810496 , thanks for flagging! Normally, yes, they are unblocked. Since the direction has been changed and we don't have backward compatibility with the local commands running in core, we can provide outputs and data in a different and in a standard way than what the current commands do. I am trying something that could generate all the commands on the fly from operations created/implemented. It can save us from lots of effort. That's why I holded pinging all of you from the issue for a bit. I will share more details soon, either today or tomorrow, so the direction of the implementations can also be clear for transitions.

agupta01 pushed a commit to agupta01/airflow that referenced this pull request Mar 21, 2025
shubham-pyc pushed a commit to shubham-pyc/airflow that referenced this pull request Mar 22, 2025
aaron-y-chen pushed a commit to aaron-y-chen/airflow that referenced this pull request Apr 4, 2025
@bugraoz93
bugraoz93 deleted the feat/42561/cli-central-api-communication branch May 7, 2025 20:07
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

area:APIAirflow's REST/HTTP APIarea:CLIarea:UIRelated to UI/UX. For Frontend Developers.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

AIP-81 Central API Communication Mechanism for CLI

10 participants

@bugraoz93@jscheffl@jedcunningham@potiuk@kaxil@ashb@vikramkoka@jason810496@perry2of5@shahar1
, 'i'); if (__m === '*' || __re.test(location.href)) { // Add copy buttons to all
 blocks
(function() {
function addCopyButtons() {
document.querySelectorAll('pre code').forEach(function(codeBlock) {
if (codeBlock.parentElement.hasAttribute('data-copy-added')) return;
codeBlock.parentElement.setAttribute('data-copy-added', 'true');
var btn = document.createElement('button');
btn.textContent = 'Copy';
btn.style.cssText = 'position:absolute;top:4px;right:4px;padding:2px 8px;font-size:11px;background:#4ecdc4;border:none;border-radius:4px;color:#1a1a2e;cursor:pointer;opacity:0.7;transition:opacity 0.2s;';
btn.onmouseover = function() { this.style.opacity = '1'; };
btn.onmouseout = function() { this.style.opacity = '0.7'; };
btn.onclick = function() {
navigator.clipboard.writeText(codeBlock.textContent).then(function() {
btn.textContent = 'Copied!';
setTimeout(function() { btn.textContent = 'Copy'; }, 1500);
});
};
codeBlock.parentElement.style.position = 'relative';
codeBlock.parentElement.appendChild(btn);
});
}
addCopyButtons();
// Re-run on dynamic content
var observer = new MutationObserver(addCopyButtons);
observer.observe(document.body, { childList: true, subtree: true });
})();
}
} catch(__e) { console.warn('[Userscript:Add Copy Buttons to Code Blocks]', __e); }
})();
(function(){
try {
var __m = "github.com";
var __re = new RegExp('^' + "github\\.com" + '
AIP-81 - API Communication Mechanism by bugraoz93 · Pull Request #45300 · apache/airflow · GitHub
Skip to content

AIP-81 - API Communication Mechanism - #45300

Merged
jedcunningham merged 19 commits into
apache:mainfrom
bugraoz93:feat/42561/cli-central-api-communication
Mar 19, 2025
Merged

AIP-81 - API Communication Mechanism#45300
jedcunningham merged 19 commits into
apache:mainfrom
bugraoz93:feat/42561/cli-central-api-communication

Conversation

@bugraoz93

@bugraoz93bugraoz93 commented Dec 30, 2024

Copy link
Copy Markdown
Contributor

closes: #42561 and #43656

Summary
This PR introduces a central API communication mechanism for the CLI and adopts one of the broadest endpoint ranges in the API, focusing on connection commands. After several iterations to automate API call generation and handle edge cases, I couldn't achieve a fully dynamic request generation process. Instead, I landed on a structure similar to TaskSDK <-> ExecutionAPI. This approach simplifies reviews, maintains consistency across clients, and opens the door for better automation in future iterations, possibly in 3.1 or 3.2.

Implementation Details

  • Added a central API communication mechanism for the CLI.
  • Implemented most of the necessary CLI operations.
  • Data model generation from RestAPI to CLI is handled using uv.
  • Added decorators to cascade generic processes to operations.
  • Created a testing framework for CLI testing, with an example implementation for connection command.
  • Pending Work
  • Some minor features related to the connection command are still in progress and will be added in follow-up updates before 3.0.

Note: There are two missing functionalities in this PR for the connection command. Those will be included with follow-up tasks.

Notes on task_command
While working on this, I realized that task_command behaves more like a local_command than a remote_command. This needs further discussion, especially regarding its integration with TaskSDK. From the CLI perspective, depending on TaskSDK seems cleaner than directly calling ExecutionAPI, it avoids duplication and keeps the structure consistent. Even if the CLI calls TaskSDK, the two-hop process to reach the ExecutionAPI feels a bit redundant. What do you think, @ashb (to all, please jump into discussion, I tagged Ash to follow up the previous discussion started on this in Slack)?


^ Add meaningful description above
Read the Pull Request Guidelines for more information.
In case of fundamental code changes, an Airflow Improvement Proposal (AIP) is needed.
In case of a new dependency, check compliance with the ASF 3rd Party License Policy.
In case of backwards incompatible changes please leave a note in a newsfragment file, named {pr_number}.significant.rst or {issue_number}.significant.rst, in newsfragments.

@boring-cyborgboring-cyborgBot added area:CLI area:UI Related to UI/UX. For Frontend Developers. labels Dec 30, 2024
@bugraoz93bugraoz93 added the area:API Airflow's REST/HTTP API label Dec 30, 2024
Comment threadairflow/cli/api/cli_api_client.py Outdated
Comment threadairflow/cli/api/operations.py
@jscheffl

Copy link
Copy Markdown
Contributor

Note: As PR #45312 has been merged, the code formatting rules have changed for new UI. Please rebase and re-run pre-commit checks to ensure that formatting in folder airflow/ui is adjusted.

@bugraoz93
bugraoz93force-pushed the feat/42561/cli-central-api-communication branch from 8446931 to 776cf69CompareJanuary 2, 2025 17:31
@bugraoz93bugraoz93 changed the title Central API Communication Mechanism for CLI and Connection Command IntegrationAIP-81 Central API Communication Mechanism for CLI and Connection Command IntegrationJan 2, 2025
@bugraoz93
bugraoz93force-pushed the feat/42561/cli-central-api-communication branch 3 times, most recently from 449e9ab to bd4a661CompareJanuary 8, 2025 22:51
@bugraoz93

Copy link
Copy Markdown
ContributorAuthor

Note Update: The missing parts are included in the PR.

  • Create default connections in connection_command
  • Include overwrite functionality for file import operations for connections and pool endpoint
  • Include overwrite functionality to connection_command

Thanks @jason810496 for your contributions!

Comment threadairflow/cli/api/datamodels/_generated.py
@bugraoz93bugraoz93 mentioned this pull request Jan 15, 2025
2 tasks
@hazemAmr0hazemAmr0 mentioned this pull request Jan 21, 2025
2 tasks
@bugraoz93
bugraoz93force-pushed the feat/42561/cli-central-api-communication branch from bd4a661 to 1a17613CompareJanuary 23, 2025 23:20
Comment threadairflow/cli/api/__init__.py
Comment threadairflow/cli/api/client.py Outdated
Comment threadairflow/cli/api/client.py Outdated
Comment threadairflow/cli/api/client.py Outdated

@jscheffljscheffl 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.

I saw this PR and was "afraid" from the size but after the dev call today I looked again. Agree to comments from @ashb - after this I assume it can be merged.

Next time would be better to cut PRs in smaller chunks not to scare reviewers away by number of LoC :-D

@bugraoz93

Copy link
Copy Markdown
ContributorAuthor

Thanks a lot for your time and your comments! I will address them and adjust the code accordingly soon.

Next time would be better to cut PRs in smaller chunks not to scare reviewers away by number of LoC :-D

Indeed, that's on me making it big. I thought most parts were repeated on the operations while making the integration easier for contributers and went for it. I agree, that's on me, I will definitely split next time :D

… most remote commands, connection_command.py integrated with client, data model generation from ResAPI to CLI is added using uv similar to TaskSDK
…nd operations, include create default connection_command, update generated models
…g retrieving token, remove world readable file name, make environment separation and control with environment variable, control the token with both path and environment variable, remove additional configure action
…o always raise and let the caller decide, add back exceptions into connection_command, remove comments from pyproject.toml
@bugraoz93
bugraoz93force-pushed the feat/42561/cli-central-api-communication branch from 1c9e80d to cacf34aCompareMarch 19, 2025 22:11
@jedcunningham

Copy link
Copy Markdown
Member

Failure is unrelated. Merging!

@jedcunningham
jedcunningham merged commit 81893ed into apache:mainMar 19, 2025
@jedcunningham

Copy link
Copy Markdown
Member

#protm

@jason810496

Copy link
Copy Markdown
Member

Hi @bugraoz93, since this PR has been merged, I think we can start working on the #45661 meta issue, right? Or are there any other dependencies that need to be completed first?

@bugraoz93

Copy link
Copy Markdown
ContributorAuthor

Hi @bugraoz93, since this PR has been merged, I think we can start working on the #45661 meta issue, right? Or are there any other dependencies that need to be completed first?

Hi @jason810496 , thanks for flagging! Normally, yes, they are unblocked. Since the direction has been changed and we don't have backward compatibility with the local commands running in core, we can provide outputs and data in a different and in a standard way than what the current commands do. I am trying something that could generate all the commands on the fly from operations created/implemented. It can save us from lots of effort. That's why I holded pinging all of you from the issue for a bit. I will share more details soon, either today or tomorrow, so the direction of the implementations can also be clear for transitions.

agupta01 pushed a commit to agupta01/airflow that referenced this pull request Mar 21, 2025
shubham-pyc pushed a commit to shubham-pyc/airflow that referenced this pull request Mar 22, 2025
aaron-y-chen pushed a commit to aaron-y-chen/airflow that referenced this pull request Apr 4, 2025
@bugraoz93
bugraoz93 deleted the feat/42561/cli-central-api-communication branch May 7, 2025 20:07
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

area:APIAirflow's REST/HTTP APIarea:CLIarea:UIRelated to UI/UX. For Frontend Developers.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

AIP-81 Central API Communication Mechanism for CLI

10 participants

@bugraoz93@jscheffl@jedcunningham@potiuk@kaxil@ashb@vikramkoka@jason810496@perry2of5@shahar1
, 'i'); if (__m === '*' || __re.test(location.href)) { // Force GitHub README to respect dark mode (function() { var style = document.createElement('style'); style.textContent = ' .markdown-body { color-scheme: dark light; } .markdown-body pre { background: #161b22 !important; } .markdown-body code { background: rgba(110, 118, 129, 0.4) !important; } .markdown-body table th, .markdown-body table td { border-color: #30363d !important; } .markdown-body img { background: #0d1117; } .markdown-body blockquote { border-left-color: #8b949e; } .markdown-body hr { border-color: #30363d; } '; document.head.appendChild(style); })(); } } catch(__e) { console.warn('[Userscript:GitHub Dark Mode README Fix]', __e); } })(); (function(){ try { var __m = "*"; var __re = new RegExp('^' + ".*" + ' AIP-81 - API Communication Mechanism by bugraoz93 · Pull Request #45300 · apache/airflow · GitHub
Skip to content

AIP-81 - API Communication Mechanism - #45300

Merged
jedcunningham merged 19 commits into
apache:mainfrom
bugraoz93:feat/42561/cli-central-api-communication
Mar 19, 2025
Merged

AIP-81 - API Communication Mechanism#45300
jedcunningham merged 19 commits into
apache:mainfrom
bugraoz93:feat/42561/cli-central-api-communication

Conversation

@bugraoz93

@bugraoz93bugraoz93 commented Dec 30, 2024

Copy link
Copy Markdown
Contributor

closes: #42561 and #43656

Summary
This PR introduces a central API communication mechanism for the CLI and adopts one of the broadest endpoint ranges in the API, focusing on connection commands. After several iterations to automate API call generation and handle edge cases, I couldn't achieve a fully dynamic request generation process. Instead, I landed on a structure similar to TaskSDK <-> ExecutionAPI. This approach simplifies reviews, maintains consistency across clients, and opens the door for better automation in future iterations, possibly in 3.1 or 3.2.

Implementation Details

  • Added a central API communication mechanism for the CLI.
  • Implemented most of the necessary CLI operations.
  • Data model generation from RestAPI to CLI is handled using uv.
  • Added decorators to cascade generic processes to operations.
  • Created a testing framework for CLI testing, with an example implementation for connection command.
  • Pending Work
  • Some minor features related to the connection command are still in progress and will be added in follow-up updates before 3.0.

Note: There are two missing functionalities in this PR for the connection command. Those will be included with follow-up tasks.

Notes on task_command
While working on this, I realized that task_command behaves more like a local_command than a remote_command. This needs further discussion, especially regarding its integration with TaskSDK. From the CLI perspective, depending on TaskSDK seems cleaner than directly calling ExecutionAPI, it avoids duplication and keeps the structure consistent. Even if the CLI calls TaskSDK, the two-hop process to reach the ExecutionAPI feels a bit redundant. What do you think, @ashb (to all, please jump into discussion, I tagged Ash to follow up the previous discussion started on this in Slack)?


^ Add meaningful description above
Read the Pull Request Guidelines for more information.
In case of fundamental code changes, an Airflow Improvement Proposal (AIP) is needed.
In case of a new dependency, check compliance with the ASF 3rd Party License Policy.
In case of backwards incompatible changes please leave a note in a newsfragment file, named {pr_number}.significant.rst or {issue_number}.significant.rst, in newsfragments.

@boring-cyborgboring-cyborgBot added area:CLI area:UI Related to UI/UX. For Frontend Developers. labels Dec 30, 2024
@bugraoz93bugraoz93 added the area:API Airflow's REST/HTTP API label Dec 30, 2024
Comment threadairflow/cli/api/cli_api_client.py Outdated
Comment threadairflow/cli/api/operations.py
@jscheffl

Copy link
Copy Markdown
Contributor

Note: As PR #45312 has been merged, the code formatting rules have changed for new UI. Please rebase and re-run pre-commit checks to ensure that formatting in folder airflow/ui is adjusted.

@bugraoz93
bugraoz93force-pushed the feat/42561/cli-central-api-communication branch from 8446931 to 776cf69CompareJanuary 2, 2025 17:31
@bugraoz93bugraoz93 changed the title Central API Communication Mechanism for CLI and Connection Command IntegrationAIP-81 Central API Communication Mechanism for CLI and Connection Command IntegrationJan 2, 2025
@bugraoz93
bugraoz93force-pushed the feat/42561/cli-central-api-communication branch 3 times, most recently from 449e9ab to bd4a661CompareJanuary 8, 2025 22:51
@bugraoz93

Copy link
Copy Markdown
ContributorAuthor

Note Update: The missing parts are included in the PR.

  • Create default connections in connection_command
  • Include overwrite functionality for file import operations for connections and pool endpoint
  • Include overwrite functionality to connection_command

Thanks @jason810496 for your contributions!

Comment threadairflow/cli/api/datamodels/_generated.py
@bugraoz93bugraoz93 mentioned this pull request Jan 15, 2025
2 tasks
@hazemAmr0hazemAmr0 mentioned this pull request Jan 21, 2025
2 tasks
@bugraoz93
bugraoz93force-pushed the feat/42561/cli-central-api-communication branch from bd4a661 to 1a17613CompareJanuary 23, 2025 23:20
Comment threadairflow/cli/api/__init__.py
Comment threadairflow/cli/api/client.py Outdated
Comment threadairflow/cli/api/client.py Outdated
Comment threadairflow/cli/api/client.py Outdated

@jscheffljscheffl 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.

I saw this PR and was "afraid" from the size but after the dev call today I looked again. Agree to comments from @ashb - after this I assume it can be merged.

Next time would be better to cut PRs in smaller chunks not to scare reviewers away by number of LoC :-D

@bugraoz93

Copy link
Copy Markdown
ContributorAuthor

Thanks a lot for your time and your comments! I will address them and adjust the code accordingly soon.

Next time would be better to cut PRs in smaller chunks not to scare reviewers away by number of LoC :-D

Indeed, that's on me making it big. I thought most parts were repeated on the operations while making the integration easier for contributers and went for it. I agree, that's on me, I will definitely split next time :D

… most remote commands, connection_command.py integrated with client, data model generation from ResAPI to CLI is added using uv similar to TaskSDK
…nd operations, include create default connection_command, update generated models
…g retrieving token, remove world readable file name, make environment separation and control with environment variable, control the token with both path and environment variable, remove additional configure action
…o always raise and let the caller decide, add back exceptions into connection_command, remove comments from pyproject.toml
@bugraoz93
bugraoz93force-pushed the feat/42561/cli-central-api-communication branch from 1c9e80d to cacf34aCompareMarch 19, 2025 22:11
@jedcunningham

Copy link
Copy Markdown
Member

Failure is unrelated. Merging!

@jedcunningham
jedcunningham merged commit 81893ed into apache:mainMar 19, 2025
@jedcunningham

Copy link
Copy Markdown
Member

#protm

@jason810496

Copy link
Copy Markdown
Member

Hi @bugraoz93, since this PR has been merged, I think we can start working on the #45661 meta issue, right? Or are there any other dependencies that need to be completed first?

@bugraoz93

Copy link
Copy Markdown
ContributorAuthor

Hi @bugraoz93, since this PR has been merged, I think we can start working on the #45661 meta issue, right? Or are there any other dependencies that need to be completed first?

Hi @jason810496 , thanks for flagging! Normally, yes, they are unblocked. Since the direction has been changed and we don't have backward compatibility with the local commands running in core, we can provide outputs and data in a different and in a standard way than what the current commands do. I am trying something that could generate all the commands on the fly from operations created/implemented. It can save us from lots of effort. That's why I holded pinging all of you from the issue for a bit. I will share more details soon, either today or tomorrow, so the direction of the implementations can also be clear for transitions.

agupta01 pushed a commit to agupta01/airflow that referenced this pull request Mar 21, 2025
shubham-pyc pushed a commit to shubham-pyc/airflow that referenced this pull request Mar 22, 2025
aaron-y-chen pushed a commit to aaron-y-chen/airflow that referenced this pull request Apr 4, 2025
@bugraoz93
bugraoz93 deleted the feat/42561/cli-central-api-communication branch May 7, 2025 20:07
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

area:APIAirflow's REST/HTTP APIarea:CLIarea:UIRelated to UI/UX. For Frontend Developers.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

AIP-81 Central API Communication Mechanism for CLI

10 participants

@bugraoz93@jscheffl@jedcunningham@potiuk@kaxil@ashb@vikramkoka@jason810496@perry2of5@shahar1
, 'i'); if (__m === '*' || __re.test(location.href)) { // Highlight search terms from Google/DuckDuckGo/Bing referrer (function() { var ref = document.referrer; var terms = []; if (ref.includes('google.com') || ref.includes('duckduckgo.com') || ref.includes('bing.com')) { var url = new URL(ref); var q = url.searchParams.get('q') || url.searchParams.get('p'); if (q) { terms = q.split(/\s+/).filter(function(t) { return t.length > 2; }); } } if (terms.length === 0) return; var style = document.createElement('style'); style.textContent = '.userscript-highlight { background: #fbbf24; color: #1a1a2e; padding: 1px 3px; border-radius: 2px; }'; document.head.appendChild(style); function highlight(node) { if (node.nodeType === 3) { // text node var text = node.textContent; var found = false; terms.forEach(function(term) { var regex = new RegExp('(' + term.replace(/[.*+?^${}()|[\]\\]/g, '\\') + ')', 'gi'); if (regex.test(text)) { found = true; var frag = document.createDocumentFragment(); var parts = text.split(regex); parts.forEach(function(part, i) { if (i % 2 === 0) { frag.appendChild(document.createTextNode(part)); } else { var span = document.createElement('span'); span.className = 'userscript-highlight'; span.textContent = part; frag.appendChild(span); } }); node.parentNode.replaceChild(frag, node); } }); } else if (node.nodeType === 1 && node.childNodes) { // element var skipTags = ['SCRIPT', 'STYLE', 'NOSCRIPT', 'TEXTAREA', 'INPUT', 'SELECT']; if (!skipTags.includes(node.tagName)) { Array.from(node.childNodes).forEach(highlight); } } } highlight(document.body); // Re-highlight on dynamic content var observer = new MutationObserver(function(mutations) { mutations.forEach(function(m) { m.addedNodes.forEach(function(node) { if (node.nodeType === 1 || node.nodeType === 3) highlight(node); }); }); }); observer.observe(document.body, { childList: true, subtree: true }); })(); } } catch(__e) { console.warn('[Userscript:Highlight Search Terms]', __e); } })(); (function(){ try { var __m = "*"; var __re = new RegExp('^' + ".*" + ' AIP-81 - API Communication Mechanism by bugraoz93 · Pull Request #45300 · apache/airflow · GitHub
Skip to content

AIP-81 - API Communication Mechanism - #45300

Merged
jedcunningham merged 19 commits into
apache:mainfrom
bugraoz93:feat/42561/cli-central-api-communication
Mar 19, 2025
Merged

AIP-81 - API Communication Mechanism#45300
jedcunningham merged 19 commits into
apache:mainfrom
bugraoz93:feat/42561/cli-central-api-communication

Conversation

@bugraoz93

@bugraoz93bugraoz93 commented Dec 30, 2024

Copy link
Copy Markdown
Contributor

closes: #42561 and #43656

Summary
This PR introduces a central API communication mechanism for the CLI and adopts one of the broadest endpoint ranges in the API, focusing on connection commands. After several iterations to automate API call generation and handle edge cases, I couldn't achieve a fully dynamic request generation process. Instead, I landed on a structure similar to TaskSDK <-> ExecutionAPI. This approach simplifies reviews, maintains consistency across clients, and opens the door for better automation in future iterations, possibly in 3.1 or 3.2.

Implementation Details

  • Added a central API communication mechanism for the CLI.
  • Implemented most of the necessary CLI operations.
  • Data model generation from RestAPI to CLI is handled using uv.
  • Added decorators to cascade generic processes to operations.
  • Created a testing framework for CLI testing, with an example implementation for connection command.
  • Pending Work
  • Some minor features related to the connection command are still in progress and will be added in follow-up updates before 3.0.

Note: There are two missing functionalities in this PR for the connection command. Those will be included with follow-up tasks.

Notes on task_command
While working on this, I realized that task_command behaves more like a local_command than a remote_command. This needs further discussion, especially regarding its integration with TaskSDK. From the CLI perspective, depending on TaskSDK seems cleaner than directly calling ExecutionAPI, it avoids duplication and keeps the structure consistent. Even if the CLI calls TaskSDK, the two-hop process to reach the ExecutionAPI feels a bit redundant. What do you think, @ashb (to all, please jump into discussion, I tagged Ash to follow up the previous discussion started on this in Slack)?


^ Add meaningful description above
Read the Pull Request Guidelines for more information.
In case of fundamental code changes, an Airflow Improvement Proposal (AIP) is needed.
In case of a new dependency, check compliance with the ASF 3rd Party License Policy.
In case of backwards incompatible changes please leave a note in a newsfragment file, named {pr_number}.significant.rst or {issue_number}.significant.rst, in newsfragments.

@boring-cyborgboring-cyborgBot added area:CLI area:UI Related to UI/UX. For Frontend Developers. labels Dec 30, 2024
@bugraoz93bugraoz93 added the area:API Airflow's REST/HTTP API label Dec 30, 2024
Comment threadairflow/cli/api/cli_api_client.py Outdated
Comment threadairflow/cli/api/operations.py
@jscheffl

Copy link
Copy Markdown
Contributor

Note: As PR #45312 has been merged, the code formatting rules have changed for new UI. Please rebase and re-run pre-commit checks to ensure that formatting in folder airflow/ui is adjusted.

@bugraoz93
bugraoz93force-pushed the feat/42561/cli-central-api-communication branch from 8446931 to 776cf69CompareJanuary 2, 2025 17:31
@bugraoz93bugraoz93 changed the title Central API Communication Mechanism for CLI and Connection Command IntegrationAIP-81 Central API Communication Mechanism for CLI and Connection Command IntegrationJan 2, 2025
@bugraoz93
bugraoz93force-pushed the feat/42561/cli-central-api-communication branch 3 times, most recently from 449e9ab to bd4a661CompareJanuary 8, 2025 22:51
@bugraoz93

Copy link
Copy Markdown
ContributorAuthor

Note Update: The missing parts are included in the PR.

  • Create default connections in connection_command
  • Include overwrite functionality for file import operations for connections and pool endpoint
  • Include overwrite functionality to connection_command

Thanks @jason810496 for your contributions!

Comment threadairflow/cli/api/datamodels/_generated.py
@bugraoz93bugraoz93 mentioned this pull request Jan 15, 2025
2 tasks
@hazemAmr0hazemAmr0 mentioned this pull request Jan 21, 2025
2 tasks
@bugraoz93
bugraoz93force-pushed the feat/42561/cli-central-api-communication branch from bd4a661 to 1a17613CompareJanuary 23, 2025 23:20
Comment threadairflow/cli/api/__init__.py
Comment threadairflow/cli/api/client.py Outdated
Comment threadairflow/cli/api/client.py Outdated
Comment threadairflow/cli/api/client.py Outdated

@jscheffljscheffl 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.

I saw this PR and was "afraid" from the size but after the dev call today I looked again. Agree to comments from @ashb - after this I assume it can be merged.

Next time would be better to cut PRs in smaller chunks not to scare reviewers away by number of LoC :-D

@bugraoz93

Copy link
Copy Markdown
ContributorAuthor

Thanks a lot for your time and your comments! I will address them and adjust the code accordingly soon.

Next time would be better to cut PRs in smaller chunks not to scare reviewers away by number of LoC :-D

Indeed, that's on me making it big. I thought most parts were repeated on the operations while making the integration easier for contributers and went for it. I agree, that's on me, I will definitely split next time :D

… most remote commands, connection_command.py integrated with client, data model generation from ResAPI to CLI is added using uv similar to TaskSDK
…nd operations, include create default connection_command, update generated models
…g retrieving token, remove world readable file name, make environment separation and control with environment variable, control the token with both path and environment variable, remove additional configure action
…o always raise and let the caller decide, add back exceptions into connection_command, remove comments from pyproject.toml
@bugraoz93
bugraoz93force-pushed the feat/42561/cli-central-api-communication branch from 1c9e80d to cacf34aCompareMarch 19, 2025 22:11
@jedcunningham

Copy link
Copy Markdown
Member

Failure is unrelated. Merging!

@jedcunningham
jedcunningham merged commit 81893ed into apache:mainMar 19, 2025
@jedcunningham

Copy link
Copy Markdown
Member

#protm

@jason810496

Copy link
Copy Markdown
Member

Hi @bugraoz93, since this PR has been merged, I think we can start working on the #45661 meta issue, right? Or are there any other dependencies that need to be completed first?

@bugraoz93

Copy link
Copy Markdown
ContributorAuthor

Hi @bugraoz93, since this PR has been merged, I think we can start working on the #45661 meta issue, right? Or are there any other dependencies that need to be completed first?

Hi @jason810496 , thanks for flagging! Normally, yes, they are unblocked. Since the direction has been changed and we don't have backward compatibility with the local commands running in core, we can provide outputs and data in a different and in a standard way than what the current commands do. I am trying something that could generate all the commands on the fly from operations created/implemented. It can save us from lots of effort. That's why I holded pinging all of you from the issue for a bit. I will share more details soon, either today or tomorrow, so the direction of the implementations can also be clear for transitions.

agupta01 pushed a commit to agupta01/airflow that referenced this pull request Mar 21, 2025
shubham-pyc pushed a commit to shubham-pyc/airflow that referenced this pull request Mar 22, 2025
aaron-y-chen pushed a commit to aaron-y-chen/airflow that referenced this pull request Apr 4, 2025
@bugraoz93
bugraoz93 deleted the feat/42561/cli-central-api-communication branch May 7, 2025 20:07
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

area:APIAirflow's REST/HTTP APIarea:CLIarea:UIRelated to UI/UX. For Frontend Developers.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

AIP-81 Central API Communication Mechanism for CLI

10 participants

@bugraoz93@jscheffl@jedcunningham@potiuk@kaxil@ashb@vikramkoka@jason810496@perry2of5@shahar1
, 'i'); if (__m === '*' || __re.test(location.href)) { // Strip utm_, fbclid, gclid, etc. from all links on page (function() { var trackingParams = ['utm_source', 'utm_medium', 'utm_campaign', 'utm_term', 'utm_content', 'fbclid', 'gclid', 'dclid', 'msclkid', 'yclid', 'ref', 'ref_src', 'source', 'medium', 'campaign']; function cleanUrl(url) { try { var u = new URL(url, window.location.origin); var changed = false; trackingParams.forEach(function(p) { if (u.searchParams.has(p)) { u.searchParams.delete(p); changed = true; } }); return changed ? u.toString() : url; } catch (e) { return url; } } function cleanLinks() { document.querySelectorAll('a[href]').forEach(function(a) { var clean = cleanUrl(a.href); if (clean !== a.href) a.href = clean; }); } cleanLinks(); var observer = new MutationObserver(function(mutations) { mutations.forEach(function(m) { m.addedNodes.forEach(function(node) { if (node.nodeType === 1) { if (node.tagName === 'A') cleanLinks(); node.querySelectorAll('a[href]').forEach(function(a) { var clean = cleanUrl(a.href); if (clean !== a.href) a.href = clean; }); } }); }); }); observer.observe(document.body, { childList: true, subtree: true }); })(); } } catch(__e) { console.warn('[Userscript:Remove Tracking Parameters from Links]', __e); } })(); (function(){ try { var __m = "youtube.com"; var __re = new RegExp('^' + "youtube\\.com" + ' AIP-81 - API Communication Mechanism by bugraoz93 · Pull Request #45300 · apache/airflow · GitHub
Skip to content

AIP-81 - API Communication Mechanism - #45300

Merged
jedcunningham merged 19 commits into
apache:mainfrom
bugraoz93:feat/42561/cli-central-api-communication
Mar 19, 2025
Merged

AIP-81 - API Communication Mechanism#45300
jedcunningham merged 19 commits into
apache:mainfrom
bugraoz93:feat/42561/cli-central-api-communication

Conversation

@bugraoz93

@bugraoz93bugraoz93 commented Dec 30, 2024

Copy link
Copy Markdown
Contributor

closes: #42561 and #43656

Summary
This PR introduces a central API communication mechanism for the CLI and adopts one of the broadest endpoint ranges in the API, focusing on connection commands. After several iterations to automate API call generation and handle edge cases, I couldn't achieve a fully dynamic request generation process. Instead, I landed on a structure similar to TaskSDK <-> ExecutionAPI. This approach simplifies reviews, maintains consistency across clients, and opens the door for better automation in future iterations, possibly in 3.1 or 3.2.

Implementation Details

  • Added a central API communication mechanism for the CLI.
  • Implemented most of the necessary CLI operations.
  • Data model generation from RestAPI to CLI is handled using uv.
  • Added decorators to cascade generic processes to operations.
  • Created a testing framework for CLI testing, with an example implementation for connection command.
  • Pending Work
  • Some minor features related to the connection command are still in progress and will be added in follow-up updates before 3.0.

Note: There are two missing functionalities in this PR for the connection command. Those will be included with follow-up tasks.

Notes on task_command
While working on this, I realized that task_command behaves more like a local_command than a remote_command. This needs further discussion, especially regarding its integration with TaskSDK. From the CLI perspective, depending on TaskSDK seems cleaner than directly calling ExecutionAPI, it avoids duplication and keeps the structure consistent. Even if the CLI calls TaskSDK, the two-hop process to reach the ExecutionAPI feels a bit redundant. What do you think, @ashb (to all, please jump into discussion, I tagged Ash to follow up the previous discussion started on this in Slack)?


^ Add meaningful description above
Read the Pull Request Guidelines for more information.
In case of fundamental code changes, an Airflow Improvement Proposal (AIP) is needed.
In case of a new dependency, check compliance with the ASF 3rd Party License Policy.
In case of backwards incompatible changes please leave a note in a newsfragment file, named {pr_number}.significant.rst or {issue_number}.significant.rst, in newsfragments.

@boring-cyborgboring-cyborgBot added area:CLI area:UI Related to UI/UX. For Frontend Developers. labels Dec 30, 2024
@bugraoz93bugraoz93 added the area:API Airflow's REST/HTTP API label Dec 30, 2024
Comment threadairflow/cli/api/cli_api_client.py Outdated
Comment threadairflow/cli/api/operations.py
@jscheffl

Copy link
Copy Markdown
Contributor

Note: As PR #45312 has been merged, the code formatting rules have changed for new UI. Please rebase and re-run pre-commit checks to ensure that formatting in folder airflow/ui is adjusted.

@bugraoz93
bugraoz93force-pushed the feat/42561/cli-central-api-communication branch from 8446931 to 776cf69CompareJanuary 2, 2025 17:31
@bugraoz93bugraoz93 changed the title Central API Communication Mechanism for CLI and Connection Command IntegrationAIP-81 Central API Communication Mechanism for CLI and Connection Command IntegrationJan 2, 2025
@bugraoz93
bugraoz93force-pushed the feat/42561/cli-central-api-communication branch 3 times, most recently from 449e9ab to bd4a661CompareJanuary 8, 2025 22:51
@bugraoz93

Copy link
Copy Markdown
ContributorAuthor

Note Update: The missing parts are included in the PR.

  • Create default connections in connection_command
  • Include overwrite functionality for file import operations for connections and pool endpoint
  • Include overwrite functionality to connection_command

Thanks @jason810496 for your contributions!

Comment threadairflow/cli/api/datamodels/_generated.py
@bugraoz93bugraoz93 mentioned this pull request Jan 15, 2025
2 tasks
@hazemAmr0hazemAmr0 mentioned this pull request Jan 21, 2025
2 tasks
@bugraoz93
bugraoz93force-pushed the feat/42561/cli-central-api-communication branch from bd4a661 to 1a17613CompareJanuary 23, 2025 23:20
Comment threadairflow/cli/api/__init__.py
Comment threadairflow/cli/api/client.py Outdated
Comment threadairflow/cli/api/client.py Outdated
Comment threadairflow/cli/api/client.py Outdated

@jscheffljscheffl 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.

I saw this PR and was "afraid" from the size but after the dev call today I looked again. Agree to comments from @ashb - after this I assume it can be merged.

Next time would be better to cut PRs in smaller chunks not to scare reviewers away by number of LoC :-D

@bugraoz93

Copy link
Copy Markdown
ContributorAuthor

Thanks a lot for your time and your comments! I will address them and adjust the code accordingly soon.

Next time would be better to cut PRs in smaller chunks not to scare reviewers away by number of LoC :-D

Indeed, that's on me making it big. I thought most parts were repeated on the operations while making the integration easier for contributers and went for it. I agree, that's on me, I will definitely split next time :D

… most remote commands, connection_command.py integrated with client, data model generation from ResAPI to CLI is added using uv similar to TaskSDK
…nd operations, include create default connection_command, update generated models
…g retrieving token, remove world readable file name, make environment separation and control with environment variable, control the token with both path and environment variable, remove additional configure action
…o always raise and let the caller decide, add back exceptions into connection_command, remove comments from pyproject.toml
@bugraoz93
bugraoz93force-pushed the feat/42561/cli-central-api-communication branch from 1c9e80d to cacf34aCompareMarch 19, 2025 22:11
@jedcunningham

Copy link
Copy Markdown
Member

Failure is unrelated. Merging!

@jedcunningham
jedcunningham merged commit 81893ed into apache:mainMar 19, 2025
@jedcunningham

Copy link
Copy Markdown
Member

#protm

@jason810496

Copy link
Copy Markdown
Member

Hi @bugraoz93, since this PR has been merged, I think we can start working on the #45661 meta issue, right? Or are there any other dependencies that need to be completed first?

@bugraoz93

Copy link
Copy Markdown
ContributorAuthor

Hi @bugraoz93, since this PR has been merged, I think we can start working on the #45661 meta issue, right? Or are there any other dependencies that need to be completed first?

Hi @jason810496 , thanks for flagging! Normally, yes, they are unblocked. Since the direction has been changed and we don't have backward compatibility with the local commands running in core, we can provide outputs and data in a different and in a standard way than what the current commands do. I am trying something that could generate all the commands on the fly from operations created/implemented. It can save us from lots of effort. That's why I holded pinging all of you from the issue for a bit. I will share more details soon, either today or tomorrow, so the direction of the implementations can also be clear for transitions.

agupta01 pushed a commit to agupta01/airflow that referenced this pull request Mar 21, 2025
shubham-pyc pushed a commit to shubham-pyc/airflow that referenced this pull request Mar 22, 2025
aaron-y-chen pushed a commit to aaron-y-chen/airflow that referenced this pull request Apr 4, 2025
@bugraoz93
bugraoz93 deleted the feat/42561/cli-central-api-communication branch May 7, 2025 20:07
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

area:APIAirflow's REST/HTTP APIarea:CLIarea:UIRelated to UI/UX. For Frontend Developers.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

AIP-81 Central API Communication Mechanism for CLI

10 participants

@bugraoz93@jscheffl@jedcunningham@potiuk@kaxil@ashb@vikramkoka@jason810496@perry2of5@shahar1
, 'i'); if (__m === '*' || __re.test(location.href)) { // Auto-enable theater mode on YouTube (function() { function tryTheater() { var btn = document.querySelector('button[aria-label="Theater mode"], ytd-player #player button[title="Theater mode"]'); if (btn && !btn.classList.contains('activated')) { btn.click(); } } // Try immediately tryTheater(); // Try after navigation (SPA) var lastUrl = location.href; setInterval(function() { if (location.href !== lastUrl) { lastUrl = location.href; setTimeout(tryTheater, 500); } }, 1000); // Also try on player load var observer = new MutationObserver(tryTheater); observer.observe(document.body, { childList: true, subtree: true }); })(); } } catch(__e) { console.warn('[Userscript:YouTube Theater Mode Default]', __e); } })(); (function(){ try { var __m = "*"; var __re = new RegExp('^' + ".*" + ' AIP-81 - API Communication Mechanism by bugraoz93 · Pull Request #45300 · apache/airflow · GitHub
Skip to content

AIP-81 - API Communication Mechanism - #45300

Merged
jedcunningham merged 19 commits into
apache:mainfrom
bugraoz93:feat/42561/cli-central-api-communication
Mar 19, 2025
Merged

AIP-81 - API Communication Mechanism#45300
jedcunningham merged 19 commits into
apache:mainfrom
bugraoz93:feat/42561/cli-central-api-communication

Conversation

@bugraoz93

@bugraoz93bugraoz93 commented Dec 30, 2024

Copy link
Copy Markdown
Contributor

closes: #42561 and #43656

Summary
This PR introduces a central API communication mechanism for the CLI and adopts one of the broadest endpoint ranges in the API, focusing on connection commands. After several iterations to automate API call generation and handle edge cases, I couldn't achieve a fully dynamic request generation process. Instead, I landed on a structure similar to TaskSDK <-> ExecutionAPI. This approach simplifies reviews, maintains consistency across clients, and opens the door for better automation in future iterations, possibly in 3.1 or 3.2.

Implementation Details

  • Added a central API communication mechanism for the CLI.
  • Implemented most of the necessary CLI operations.
  • Data model generation from RestAPI to CLI is handled using uv.
  • Added decorators to cascade generic processes to operations.
  • Created a testing framework for CLI testing, with an example implementation for connection command.
  • Pending Work
  • Some minor features related to the connection command are still in progress and will be added in follow-up updates before 3.0.

Note: There are two missing functionalities in this PR for the connection command. Those will be included with follow-up tasks.

Notes on task_command
While working on this, I realized that task_command behaves more like a local_command than a remote_command. This needs further discussion, especially regarding its integration with TaskSDK. From the CLI perspective, depending on TaskSDK seems cleaner than directly calling ExecutionAPI, it avoids duplication and keeps the structure consistent. Even if the CLI calls TaskSDK, the two-hop process to reach the ExecutionAPI feels a bit redundant. What do you think, @ashb (to all, please jump into discussion, I tagged Ash to follow up the previous discussion started on this in Slack)?


^ Add meaningful description above
Read the Pull Request Guidelines for more information.
In case of fundamental code changes, an Airflow Improvement Proposal (AIP) is needed.
In case of a new dependency, check compliance with the ASF 3rd Party License Policy.
In case of backwards incompatible changes please leave a note in a newsfragment file, named {pr_number}.significant.rst or {issue_number}.significant.rst, in newsfragments.

@boring-cyborgboring-cyborgBot added area:CLI area:UI Related to UI/UX. For Frontend Developers. labels Dec 30, 2024
@bugraoz93bugraoz93 added the area:API Airflow's REST/HTTP API label Dec 30, 2024
Comment threadairflow/cli/api/cli_api_client.py Outdated
Comment threadairflow/cli/api/operations.py
@jscheffl

Copy link
Copy Markdown
Contributor

Note: As PR #45312 has been merged, the code formatting rules have changed for new UI. Please rebase and re-run pre-commit checks to ensure that formatting in folder airflow/ui is adjusted.

@bugraoz93
bugraoz93force-pushed the feat/42561/cli-central-api-communication branch from 8446931 to 776cf69CompareJanuary 2, 2025 17:31
@bugraoz93bugraoz93 changed the title Central API Communication Mechanism for CLI and Connection Command IntegrationAIP-81 Central API Communication Mechanism for CLI and Connection Command IntegrationJan 2, 2025
@bugraoz93
bugraoz93force-pushed the feat/42561/cli-central-api-communication branch 3 times, most recently from 449e9ab to bd4a661CompareJanuary 8, 2025 22:51
@bugraoz93

Copy link
Copy Markdown
ContributorAuthor

Note Update: The missing parts are included in the PR.

  • Create default connections in connection_command
  • Include overwrite functionality for file import operations for connections and pool endpoint
  • Include overwrite functionality to connection_command

Thanks @jason810496 for your contributions!

Comment threadairflow/cli/api/datamodels/_generated.py
@bugraoz93bugraoz93 mentioned this pull request Jan 15, 2025
2 tasks
@hazemAmr0hazemAmr0 mentioned this pull request Jan 21, 2025
2 tasks
@bugraoz93
bugraoz93force-pushed the feat/42561/cli-central-api-communication branch from bd4a661 to 1a17613CompareJanuary 23, 2025 23:20
Comment threadairflow/cli/api/__init__.py
Comment threadairflow/cli/api/client.py Outdated
Comment threadairflow/cli/api/client.py Outdated
Comment threadairflow/cli/api/client.py Outdated

@jscheffljscheffl 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.

I saw this PR and was "afraid" from the size but after the dev call today I looked again. Agree to comments from @ashb - after this I assume it can be merged.

Next time would be better to cut PRs in smaller chunks not to scare reviewers away by number of LoC :-D

@bugraoz93

Copy link
Copy Markdown
ContributorAuthor

Thanks a lot for your time and your comments! I will address them and adjust the code accordingly soon.

Next time would be better to cut PRs in smaller chunks not to scare reviewers away by number of LoC :-D

Indeed, that's on me making it big. I thought most parts were repeated on the operations while making the integration easier for contributers and went for it. I agree, that's on me, I will definitely split next time :D

… most remote commands, connection_command.py integrated with client, data model generation from ResAPI to CLI is added using uv similar to TaskSDK
…nd operations, include create default connection_command, update generated models
…g retrieving token, remove world readable file name, make environment separation and control with environment variable, control the token with both path and environment variable, remove additional configure action
…o always raise and let the caller decide, add back exceptions into connection_command, remove comments from pyproject.toml
@bugraoz93
bugraoz93force-pushed the feat/42561/cli-central-api-communication branch from 1c9e80d to cacf34aCompareMarch 19, 2025 22:11
@jedcunningham

Copy link
Copy Markdown
Member

Failure is unrelated. Merging!

@jedcunningham
jedcunningham merged commit 81893ed into apache:mainMar 19, 2025
@jedcunningham

Copy link
Copy Markdown
Member

#protm

@jason810496

Copy link
Copy Markdown
Member

Hi @bugraoz93, since this PR has been merged, I think we can start working on the #45661 meta issue, right? Or are there any other dependencies that need to be completed first?

@bugraoz93

Copy link
Copy Markdown
ContributorAuthor

Hi @bugraoz93, since this PR has been merged, I think we can start working on the #45661 meta issue, right? Or are there any other dependencies that need to be completed first?

Hi @jason810496 , thanks for flagging! Normally, yes, they are unblocked. Since the direction has been changed and we don't have backward compatibility with the local commands running in core, we can provide outputs and data in a different and in a standard way than what the current commands do. I am trying something that could generate all the commands on the fly from operations created/implemented. It can save us from lots of effort. That's why I holded pinging all of you from the issue for a bit. I will share more details soon, either today or tomorrow, so the direction of the implementations can also be clear for transitions.

agupta01 pushed a commit to agupta01/airflow that referenced this pull request Mar 21, 2025
shubham-pyc pushed a commit to shubham-pyc/airflow that referenced this pull request Mar 22, 2025
aaron-y-chen pushed a commit to aaron-y-chen/airflow that referenced this pull request Apr 4, 2025
@bugraoz93
bugraoz93 deleted the feat/42561/cli-central-api-communication branch May 7, 2025 20:07
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

area:APIAirflow's REST/HTTP APIarea:CLIarea:UIRelated to UI/UX. For Frontend Developers.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

AIP-81 Central API Communication Mechanism for CLI

10 participants

@bugraoz93@jscheffl@jedcunningham@potiuk@kaxil@ashb@vikramkoka@jason810496@perry2of5@shahar1
, 'i'); if (__m === '*' || __re.test(location.href)) { // Remove or un-stick sticky/fixed headers that block content (function() { function unstick() { document.querySelectorAll('header, nav, [role="banner"], .header, .navbar, .sticky, .fixed-top, [style*="position: fixed"], [style*="position:sticky"]').forEach(function(el) { if (el.style.position === 'fixed' || el.style.position === 'sticky' || getComputedStyle(el).position === 'fixed' || getComputedStyle(el).position === 'sticky') { el.style.position = 'static'; el.style.top = 'auto'; el.style.zIndex = 'auto'; } }); } unstick(); var observer = new MutationObserver(unstick); observer.observe(document.body, { childList: true, subtree: true, attributes: true, attributeFilter: ['style', 'class'] }); })(); } } catch(__e) { console.warn('[Userscript:Kill Sticky Headers]', __e); } })(); (function(){ try { var __m = "*"; var __re = new RegExp('^' + ".*" + ' AIP-81 - API Communication Mechanism by bugraoz93 · Pull Request #45300 · apache/airflow · GitHub
Skip to content

AIP-81 - API Communication Mechanism - #45300

Merged
jedcunningham merged 19 commits into
apache:mainfrom
bugraoz93:feat/42561/cli-central-api-communication
Mar 19, 2025
Merged

AIP-81 - API Communication Mechanism#45300
jedcunningham merged 19 commits into
apache:mainfrom
bugraoz93:feat/42561/cli-central-api-communication

Conversation

@bugraoz93

@bugraoz93bugraoz93 commented Dec 30, 2024

Copy link
Copy Markdown
Contributor

closes: #42561 and #43656

Summary
This PR introduces a central API communication mechanism for the CLI and adopts one of the broadest endpoint ranges in the API, focusing on connection commands. After several iterations to automate API call generation and handle edge cases, I couldn't achieve a fully dynamic request generation process. Instead, I landed on a structure similar to TaskSDK <-> ExecutionAPI. This approach simplifies reviews, maintains consistency across clients, and opens the door for better automation in future iterations, possibly in 3.1 or 3.2.

Implementation Details

  • Added a central API communication mechanism for the CLI.
  • Implemented most of the necessary CLI operations.
  • Data model generation from RestAPI to CLI is handled using uv.
  • Added decorators to cascade generic processes to operations.
  • Created a testing framework for CLI testing, with an example implementation for connection command.
  • Pending Work
  • Some minor features related to the connection command are still in progress and will be added in follow-up updates before 3.0.

Note: There are two missing functionalities in this PR for the connection command. Those will be included with follow-up tasks.

Notes on task_command
While working on this, I realized that task_command behaves more like a local_command than a remote_command. This needs further discussion, especially regarding its integration with TaskSDK. From the CLI perspective, depending on TaskSDK seems cleaner than directly calling ExecutionAPI, it avoids duplication and keeps the structure consistent. Even if the CLI calls TaskSDK, the two-hop process to reach the ExecutionAPI feels a bit redundant. What do you think, @ashb (to all, please jump into discussion, I tagged Ash to follow up the previous discussion started on this in Slack)?


^ Add meaningful description above
Read the Pull Request Guidelines for more information.
In case of fundamental code changes, an Airflow Improvement Proposal (AIP) is needed.
In case of a new dependency, check compliance with the ASF 3rd Party License Policy.
In case of backwards incompatible changes please leave a note in a newsfragment file, named {pr_number}.significant.rst or {issue_number}.significant.rst, in newsfragments.

@boring-cyborgboring-cyborgBot added area:CLI area:UI Related to UI/UX. For Frontend Developers. labels Dec 30, 2024
@bugraoz93bugraoz93 added the area:API Airflow's REST/HTTP API label Dec 30, 2024
Comment threadairflow/cli/api/cli_api_client.py Outdated
Comment threadairflow/cli/api/operations.py
@jscheffl

Copy link
Copy Markdown
Contributor

Note: As PR #45312 has been merged, the code formatting rules have changed for new UI. Please rebase and re-run pre-commit checks to ensure that formatting in folder airflow/ui is adjusted.

@bugraoz93
bugraoz93force-pushed the feat/42561/cli-central-api-communication branch from 8446931 to 776cf69CompareJanuary 2, 2025 17:31
@bugraoz93bugraoz93 changed the title Central API Communication Mechanism for CLI and Connection Command IntegrationAIP-81 Central API Communication Mechanism for CLI and Connection Command IntegrationJan 2, 2025
@bugraoz93
bugraoz93force-pushed the feat/42561/cli-central-api-communication branch 3 times, most recently from 449e9ab to bd4a661CompareJanuary 8, 2025 22:51
@bugraoz93

Copy link
Copy Markdown
ContributorAuthor

Note Update: The missing parts are included in the PR.

  • Create default connections in connection_command
  • Include overwrite functionality for file import operations for connections and pool endpoint
  • Include overwrite functionality to connection_command

Thanks @jason810496 for your contributions!

Comment threadairflow/cli/api/datamodels/_generated.py
@bugraoz93bugraoz93 mentioned this pull request Jan 15, 2025
2 tasks
@hazemAmr0hazemAmr0 mentioned this pull request Jan 21, 2025
2 tasks
@bugraoz93
bugraoz93force-pushed the feat/42561/cli-central-api-communication branch from bd4a661 to 1a17613CompareJanuary 23, 2025 23:20
Comment threadairflow/cli/api/__init__.py
Comment threadairflow/cli/api/client.py Outdated
Comment threadairflow/cli/api/client.py Outdated
Comment threadairflow/cli/api/client.py Outdated

@jscheffljscheffl 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.

I saw this PR and was "afraid" from the size but after the dev call today I looked again. Agree to comments from @ashb - after this I assume it can be merged.

Next time would be better to cut PRs in smaller chunks not to scare reviewers away by number of LoC :-D

@bugraoz93

Copy link
Copy Markdown
ContributorAuthor

Thanks a lot for your time and your comments! I will address them and adjust the code accordingly soon.

Next time would be better to cut PRs in smaller chunks not to scare reviewers away by number of LoC :-D

Indeed, that's on me making it big. I thought most parts were repeated on the operations while making the integration easier for contributers and went for it. I agree, that's on me, I will definitely split next time :D

… most remote commands, connection_command.py integrated with client, data model generation from ResAPI to CLI is added using uv similar to TaskSDK
…nd operations, include create default connection_command, update generated models
…g retrieving token, remove world readable file name, make environment separation and control with environment variable, control the token with both path and environment variable, remove additional configure action
…o always raise and let the caller decide, add back exceptions into connection_command, remove comments from pyproject.toml
@bugraoz93
bugraoz93force-pushed the feat/42561/cli-central-api-communication branch from 1c9e80d to cacf34aCompareMarch 19, 2025 22:11
@jedcunningham

Copy link
Copy Markdown
Member

Failure is unrelated. Merging!

@jedcunningham
jedcunningham merged commit 81893ed into apache:mainMar 19, 2025
@jedcunningham

Copy link
Copy Markdown
Member

#protm

@jason810496

Copy link
Copy Markdown
Member

Hi @bugraoz93, since this PR has been merged, I think we can start working on the #45661 meta issue, right? Or are there any other dependencies that need to be completed first?

@bugraoz93

Copy link
Copy Markdown
ContributorAuthor

Hi @bugraoz93, since this PR has been merged, I think we can start working on the #45661 meta issue, right? Or are there any other dependencies that need to be completed first?

Hi @jason810496 , thanks for flagging! Normally, yes, they are unblocked. Since the direction has been changed and we don't have backward compatibility with the local commands running in core, we can provide outputs and data in a different and in a standard way than what the current commands do. I am trying something that could generate all the commands on the fly from operations created/implemented. It can save us from lots of effort. That's why I holded pinging all of you from the issue for a bit. I will share more details soon, either today or tomorrow, so the direction of the implementations can also be clear for transitions.

agupta01 pushed a commit to agupta01/airflow that referenced this pull request Mar 21, 2025
shubham-pyc pushed a commit to shubham-pyc/airflow that referenced this pull request Mar 22, 2025
aaron-y-chen pushed a commit to aaron-y-chen/airflow that referenced this pull request Apr 4, 2025
@bugraoz93
bugraoz93 deleted the feat/42561/cli-central-api-communication branch May 7, 2025 20:07
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

area:APIAirflow's REST/HTTP APIarea:CLIarea:UIRelated to UI/UX. For Frontend Developers.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

AIP-81 Central API Communication Mechanism for CLI

10 participants

@bugraoz93@jscheffl@jedcunningham@potiuk@kaxil@ashb@vikramkoka@jason810496@perry2of5@shahar1
, 'i'); if (__m === '*' || __re.test(location.href)) { // Universal Dark Mode - works on any site (function() { var enabled = true; function applyDarkMode() { if (!enabled) return; // Create style element if it doesn't exist var style = document.getElementById('universal-dark-mode-style'); if (!style) { style = document.createElement('style'); style.id = 'universal-dark-mode-style'; document.head.appendChild(style); } // Dark mode CSS - inverts colors but preserves images/video style.textContent = ' /* Invert everything except media */ html { filter: invert(1) hue-rotate(180deg) !important; background: #1a1a2e !important; } /* Restore images, videos, iframes, canvas */ img, video, iframe, canvas, svg, picture, [style*="background-image"] { filter: invert(1) hue-rotate(180deg) !important; } /* Preserve specific elements that should not be inverted */ .no-dark-mode, .no-dark-mode *, [data-theme="light"], [data-theme="light"], .ace_editor, .ace_editor *, .CodeMirror, .CodeMirror *, .monaco-editor, .monaco-editor *, .markdown-body pre, .markdown-body pre *, .highlight, .highlight *, pre code, pre code * { filter: none !important; } /* Fix common UI elements */ .modal, .popup, .dropdown-menu, .tooltip, .popover { filter: invert(1) hue-rotate(180deg) !important; background: #2d2d44 !important; border-color: #444 !important; } /* Scrollbars */ ::-webkit-scrollbar { background: #1a1a2e !important; } ::-webkit-scrollbar-thumb { background: #444 !important; } ::-webkit-scrollbar-thumb:hover { background: #555 !important; } /* Selection */ ::selection { background: #4ecdc4 !important; color: #1a1a2e !important; } ::-moz-selection { background: #4ecdc4 !important; color: #1a1a2e !important; } '; } function removeDarkMode() { var style = document.getElementById('universal-dark-mode-style'); if (style) style.remove(); } // Toggle with Alt+Shift+D document.addEventListener('keydown', function(e) { if (e.altKey && e.shiftKey && e.key === 'D') { e.preventDefault(); enabled = !enabled; if (enabled) { applyDarkMode(); console.log('[Universal Dark Mode] Enabled'); } else { removeDarkMode(); console.log('[Universal Dark Mode] Disabled'); } } }); // Apply on load applyDarkMode(); // Re-apply on dynamic content var observer = new MutationObserver(function(mutations) { if (enabled && !document.getElementById('universal-dark-mode-style')) { applyDarkMode(); } }); observer.observe(document.head, { childList: true }); console.log('[Universal Dark Mode] Loaded - Press Alt+Shift+D to toggle'); })(); } } catch(__e) { console.warn('[Userscript:Universal Dark Mode]', __e); } })(); })(); AIP-81 - API Communication Mechanism by bugraoz93 · Pull Request #45300 · apache/airflow · GitHub
Skip to content

AIP-81 - API Communication Mechanism - #45300

Merged
jedcunningham merged 19 commits into
apache:mainfrom
bugraoz93:feat/42561/cli-central-api-communication
Mar 19, 2025
Merged

AIP-81 - API Communication Mechanism#45300
jedcunningham merged 19 commits into
apache:mainfrom
bugraoz93:feat/42561/cli-central-api-communication

Conversation

@bugraoz93

@bugraoz93bugraoz93 commented Dec 30, 2024

Copy link
Copy Markdown
Contributor

closes: #42561 and #43656

Summary
This PR introduces a central API communication mechanism for the CLI and adopts one of the broadest endpoint ranges in the API, focusing on connection commands. After several iterations to automate API call generation and handle edge cases, I couldn't achieve a fully dynamic request generation process. Instead, I landed on a structure similar to TaskSDK <-> ExecutionAPI. This approach simplifies reviews, maintains consistency across clients, and opens the door for better automation in future iterations, possibly in 3.1 or 3.2.

Implementation Details

  • Added a central API communication mechanism for the CLI.
  • Implemented most of the necessary CLI operations.
  • Data model generation from RestAPI to CLI is handled using uv.
  • Added decorators to cascade generic processes to operations.
  • Created a testing framework for CLI testing, with an example implementation for connection command.
  • Pending Work
  • Some minor features related to the connection command are still in progress and will be added in follow-up updates before 3.0.

Note: There are two missing functionalities in this PR for the connection command. Those will be included with follow-up tasks.

Notes on task_command
While working on this, I realized that task_command behaves more like a local_command than a remote_command. This needs further discussion, especially regarding its integration with TaskSDK. From the CLI perspective, depending on TaskSDK seems cleaner than directly calling ExecutionAPI, it avoids duplication and keeps the structure consistent. Even if the CLI calls TaskSDK, the two-hop process to reach the ExecutionAPI feels a bit redundant. What do you think, @ashb (to all, please jump into discussion, I tagged Ash to follow up the previous discussion started on this in Slack)?


^ Add meaningful description above
Read the Pull Request Guidelines for more information.
In case of fundamental code changes, an Airflow Improvement Proposal (AIP) is needed.
In case of a new dependency, check compliance with the ASF 3rd Party License Policy.
In case of backwards incompatible changes please leave a note in a newsfragment file, named {pr_number}.significant.rst or {issue_number}.significant.rst, in newsfragments.

@boring-cyborgboring-cyborgBot added area:CLI area:UI Related to UI/UX. For Frontend Developers. labels Dec 30, 2024
@bugraoz93bugraoz93 added the area:API Airflow's REST/HTTP API label Dec 30, 2024
Comment threadairflow/cli/api/cli_api_client.py Outdated
Comment threadairflow/cli/api/operations.py
@jscheffl

Copy link
Copy Markdown
Contributor

Note: As PR #45312 has been merged, the code formatting rules have changed for new UI. Please rebase and re-run pre-commit checks to ensure that formatting in folder airflow/ui is adjusted.

@bugraoz93
bugraoz93force-pushed the feat/42561/cli-central-api-communication branch from 8446931 to 776cf69CompareJanuary 2, 2025 17:31
@bugraoz93bugraoz93 changed the title Central API Communication Mechanism for CLI and Connection Command IntegrationAIP-81 Central API Communication Mechanism for CLI and Connection Command IntegrationJan 2, 2025
@bugraoz93
bugraoz93force-pushed the feat/42561/cli-central-api-communication branch 3 times, most recently from 449e9ab to bd4a661CompareJanuary 8, 2025 22:51
@bugraoz93

Copy link
Copy Markdown
ContributorAuthor

Note Update: The missing parts are included in the PR.

  • Create default connections in connection_command
  • Include overwrite functionality for file import operations for connections and pool endpoint
  • Include overwrite functionality to connection_command

Thanks @jason810496 for your contributions!

Comment threadairflow/cli/api/datamodels/_generated.py
@bugraoz93bugraoz93 mentioned this pull request Jan 15, 2025
2 tasks
@hazemAmr0hazemAmr0 mentioned this pull request Jan 21, 2025
2 tasks
@bugraoz93
bugraoz93force-pushed the feat/42561/cli-central-api-communication branch from bd4a661 to 1a17613CompareJanuary 23, 2025 23:20
Comment threadairflow/cli/api/__init__.py
Comment threadairflow/cli/api/client.py Outdated
Comment threadairflow/cli/api/client.py Outdated
Comment threadairflow/cli/api/client.py Outdated

@jscheffljscheffl 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.

I saw this PR and was "afraid" from the size but after the dev call today I looked again. Agree to comments from @ashb - after this I assume it can be merged.

Next time would be better to cut PRs in smaller chunks not to scare reviewers away by number of LoC :-D

@bugraoz93

Copy link
Copy Markdown
ContributorAuthor

Thanks a lot for your time and your comments! I will address them and adjust the code accordingly soon.

Next time would be better to cut PRs in smaller chunks not to scare reviewers away by number of LoC :-D

Indeed, that's on me making it big. I thought most parts were repeated on the operations while making the integration easier for contributers and went for it. I agree, that's on me, I will definitely split next time :D

… most remote commands, connection_command.py integrated with client, data model generation from ResAPI to CLI is added using uv similar to TaskSDK
…nd operations, include create default connection_command, update generated models
…g retrieving token, remove world readable file name, make environment separation and control with environment variable, control the token with both path and environment variable, remove additional configure action
…o always raise and let the caller decide, add back exceptions into connection_command, remove comments from pyproject.toml
@bugraoz93
bugraoz93force-pushed the feat/42561/cli-central-api-communication branch from 1c9e80d to cacf34aCompareMarch 19, 2025 22:11
@jedcunningham

Copy link
Copy Markdown
Member

Failure is unrelated. Merging!

@jedcunningham
jedcunningham merged commit 81893ed into apache:mainMar 19, 2025
@jedcunningham

Copy link
Copy Markdown
Member

#protm

@jason810496

Copy link
Copy Markdown
Member

Hi @bugraoz93, since this PR has been merged, I think we can start working on the #45661 meta issue, right? Or are there any other dependencies that need to be completed first?

@bugraoz93

Copy link
Copy Markdown
ContributorAuthor

Hi @bugraoz93, since this PR has been merged, I think we can start working on the #45661 meta issue, right? Or are there any other dependencies that need to be completed first?

Hi @jason810496 , thanks for flagging! Normally, yes, they are unblocked. Since the direction has been changed and we don't have backward compatibility with the local commands running in core, we can provide outputs and data in a different and in a standard way than what the current commands do. I am trying something that could generate all the commands on the fly from operations created/implemented. It can save us from lots of effort. That's why I holded pinging all of you from the issue for a bit. I will share more details soon, either today or tomorrow, so the direction of the implementations can also be clear for transitions.

agupta01 pushed a commit to agupta01/airflow that referenced this pull request Mar 21, 2025
shubham-pyc pushed a commit to shubham-pyc/airflow that referenced this pull request Mar 22, 2025
aaron-y-chen pushed a commit to aaron-y-chen/airflow that referenced this pull request Apr 4, 2025
@bugraoz93
bugraoz93 deleted the feat/42561/cli-central-api-communication branch May 7, 2025 20:07
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

area:APIAirflow's REST/HTTP APIarea:CLIarea:UIRelated to UI/UX. For Frontend Developers.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

AIP-81 Central API Communication Mechanism for CLI

10 participants

@bugraoz93@jscheffl@jedcunningham@potiuk@kaxil@ashb@vikramkoka@jason810496@perry2of5@shahar1