> As far as your statement “we should be able to see the results of the tests on your PR **before** we merge it”, what is the best way to communicate these results in a PR? Provide logs showing successful execution as part of the PR?
> The way I understand it then, our plan would be to run the system tests prior to our own commits to our Airflow fork and prior to PR, and also to have them running as part of our CI process for SAS Studio API itself. > When you say “provide a tool for the release manager to check that the status of tests is green”, do you have an example of what this would look like? (ie are there any other providers which have done this so that we could model it in a similar manner) I mean to have a site, publicly available where the tests are automatically executed when a PR is created and gets merged where we can see the status. of last main, It does not have to be always after main is merged - it can be when "service" changes are merged to main - or regularly (every day or week) - but it has to be fresh and allow to automatically check (via API) what is the status and which commit it was based on. This should be automatable. This should be not only for your changes but also for changes coming from the community - if only you would be running it for your commits and it is not open for contributions, then it makes no sense to have it as community provider whatsoever. In this case it is much better for you to run it all (since you have full control over the release process and testing). So merges to your code might also come up from anyone and committers might merge and approve it (not yours) - and it might break the system tests, in which case it's on you to fix it. Note that you have no influence on approval and merges - this is the ASF process that requires that only committers have those rights. For example when we decide that we make a global change in all providers (we did), this might break system tests of yours. But it will not hold us back from doing this change - you will have to fix your system tests in order to get that released (and we will have to approve and merge the change fixing it). We cannot afford your provider to hold changes for all others. This is quite a bit of a difficult position for you, because - for example - merges and global changes of code in airflow might break things for your tests as well, and you will have to fix them to get the provider released. That's what makes the commitment from your side much more "serious" than you currently think. But this is the price to pay for being a "community" code. Basically what we are asking for is equivalent to this (there is also the API available to query it): https://github.com/apache/airflow/actions/- but provided by your system - for changes relevant to your provider. Fully maintained by you. As mentioned before - this is a new idea for new providers to follow, and some of the big providers are working to get it up and running. And it needs quite a bit of "hashing out the details" - so whatever you can propose from your side to fulfill that - we can consider. This is what the Amazon and Google team (for example) are working on right now - I saw some previews of that and we will work on this in the coming monhts though, so it is not available yet. And we will ask others to follow. Just one - very blunt - statement, If your only reason to submit it to the community providers is to increase the discoverability and visibility, then this is a very wrong reason. You will likely have an order of magnitude more overhead on keeping up with our expectations here than you have when managing it on your own - because our bar is getting really high for new providers. This is in a stark contrast with making a PR to the ecosystem page - and keeping to your own release process and expectations. J. On Tue, Dec 20, 2022 at 10:21 PM Andrew Shakinovsky <andrew.shakinov...@sas.com.invalid> wrote: > Jarek, > > Thank you for your response, and for clarifying what you meant with > regards to testing. Yes it actually makes more sense for us to execute > these tests ourselves within our own infrastructure, and we would not > expect the community to expend any effort in this regard. > > I’ve seen the documentation on the system tests that you pointed out, and > I will make some updates on our end to comply. > > As far as your statement “we should be able to see the results of the > tests on your PR **before** we merge it”, what is the best way to > communicate these results in a PR? Provide logs showing successful > execution as part of the PR? > > The way I understand it then, our plan would be to run the system tests > prior to our own commits to our Airflow fork and prior to PR, and also to > have them running as part of our CI process for SAS Studio API itself. > > When you say “provide a tool for the release manager to check that the > status of tests is green”, do you have an example of what this would look > like? (ie are there any other providers which have done this so that we > could model it in a similar manner) > > > > Thank you for your time. > > > > *From: *Jarek Potiuk <ja...@potiuk.com> > *Date: *Tuesday, December 20, 2022 at 3:38 PM > *To: *dev@airflow.apache.org <dev@airflow.apache.org> > *Subject: *Re: New provider for SAS > > *EXTERNAL* > > > As far as community testing is concerned, SAS provides trial offerings: > https://www.sas.com/en_us/trials.html, so operator can be tested by > anyone. > > > > This is not what we are asking for and it is not nearly enough under the > new approach we have for providers. > > > > We are asking for those who submit a provider (service owners) to commit > to run the test in their infrastructure and provide a tool for the release > manager to check that teh status of tests is green. If the status will be > "red" - tests failing or not executed with the version that is about to be > released, we will just not release the new provider. Full stop. No > community effort should be expected to fix any system tests failures. It's > on you. > > > > What we ask for here is: > > > > * community and release manager needs 0 effort to run the tests and check > the status of last run > > * there is an infrastructure managed by the service provider (SAS Studio > in this case)| that runs the tests and specifically it should help the > release manager to decide if the provider is ready to be released > > * any problems there (failing tests, missing dependencies, dependencies > conflicting with other providers - will have to be solved by SAS Studio > team - if they won't - we will not release the provider. There will be no > expectations we - as a community will fix them. > > * all the infrastructure and test running has to be proven and working > before we accept the PR of merging the new provider - so basically your PR > will have to have system tests implemented and those system tests should be > running in your infrastructure, and we should be able to see the results of > the tests on your PR **before** we merge it. > > > > I just want to make crystal clear that we are now expecting any new > external service providers to comply with this process. And you need to be > aware that the PR will not be accepted before the infra and harness is in > place - following AIP-47 > https://cwiki.apache.org/confluence/display/AIRFLOW/AIP-47+New+design+of+Airflow+System+Tests > <https://nam02.safelinks.protection.outlook.com/?url=https%3A%2F%2Fcwiki.apache.org%2Fconfluence%2Fdisplay%2FAIRFLOW%2FAIP-47%2BNew%2Bdesign%2Bof%2BAirflow%2BSystem%2BTests&data=05%7C01%7CAndrew.Shakinovsky%40sas.com%7C9e56741a86d0499a83cc08dae2ca244d%7Cb1c14d5c362545b3a4309552373a0c2f%7C0%7C0%7C638071655181818447%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C3000%7C%7C%7C&sdata=Y5rm6Jpr0gzC4uqglmHKxhoMIRV0iJeLBWs8LrheRHY%3D&reserved=0> > > > > J. > > > > > > > > > > > > On Tue, Dec 20, 2022 at 8:02 PM Andrew Shakinovsky > <andrew.shakinov...@sas.com.invalid> wrote: > > Jarek and XD, > > > > Thank you for your responses. I have read through the other thread, and > understand the concerns with accepting a submission to the Airflow > community providers. I believe we can address these concerns, as we see > value in being included within the infrastructure. > > As I mentioned before, we plan to fully test, maintain and support our > provider, as it is in our best interests to do so. SAS has a number of > customers across the globe that extensively use Airflow in their > organizations and want to expand Airflow usage to orchestrate and schedule > SAS workloads. We want to make it as easy as possible for them, and we want > our Airflow provider to be as discoverable as possible. > > > > As far as community testing is concerned, SAS provides trial offerings: > https://www.sas.com/en_us/trials.html, so operator can be tested by > anyone. > > > > SAS Studio is an Integrated Development Environment for Data Engineers and > Data Scientists on SAS Viya. SAS Studio Flow is a component and an artifact > of SAS Studio used for building visual data pipelines in a low code > environment. The Studio Flow Airflow operator that we have developed allows > the execution SAS Studio Flows within SAS Viya infrastructure and provides > the ability to specify parameters needed for execution. Our operator is > generic and not tied to a specific Studio Flow. Further, our operator uses > only public documented SAS Viya APIs available through the SAS Developer > Portal: https://developer.sas.com/apis/rest/ > > > > Please let me know if there are additional concerns. I will submit a PR as > well so you can take a look at what we have. > > > > Thanks > > > > *From: *Jarek Potiuk <ja...@potiuk.com> > *Date: *Friday, December 16, 2022 at 1:05 PM > *To: *dev@airflow.apache.org <dev@airflow.apache.org> > *Cc: *andrew.shakinov...@sas.com.invalid > <andrew.shakinov...@sas.com.invalid> > *Subject: *Re: New provider for SAS > > You don't often get email from ja...@potiuk.com. Learn why this is > important <https://aka.ms/LearnAboutSenderIdentification> > > *EXTERNAL* > > Yep. I think the important one is that I believe (or at least this is my > proposal to address some of the concerns of the maintainers) we will be > leaning towards the approach that if we accept any submission for an > "external service" provider, then there must be a commitment from the > service provider to create and maintain "System test harness" on their own. > And the proposal is that they should implement "System Tests" in the > provider that will allow us in the community to know that their provider is > still functioning with the external service. > > > > That's pretty high bar, I think, and some of our service providers - > Google, Amazon, Databricks - are already committed to do that for their > providers (and even they took part in making the System tests harness > robust and usable in such scenarios). > > > > Staying with your "own" way of relassing and maintaining such a provider > is far easier and bears no such long time commitments, you have more > freedom and more control, while this means that it can only be listed in > the "ecosystem" part of the official Airflow documentation. > > > > Note - this is not yet a formalized and agreed policy, but I personally > think it is one that has the right balance of service providers needs and > community expectations. > > > > J. > > > > > > > > On Fri, Dec 16, 2022 at 4:38 PM Xiaodong Deng <xdd...@apache.org> wrote: > > Hi Andrew, > > > > Thanks a lot for the email and your interest to make the contribution. > > Earlier we did have a similar conversation for another potential provider, > at https://lists.apache.org/thread/1gtw5vyypxh0p72wh4dss7cllcvhgh01 > <https://nam02.safelinks.protection.outlook.com/?url=https%3A%2F%2Flists.apache.org%2Fthread%2F1gtw5vyypxh0p72wh4dss7cllcvhgh01&data=05%7C01%7CAndrew.Shakinovsky%40sas.com%7C9e56741a86d0499a83cc08dae2ca244d%7Cb1c14d5c362545b3a4309552373a0c2f%7C0%7C0%7C638071655181974685%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C3000%7C%7C%7C&sdata=7Q918%2FWUzVeBFNNCJR3ZLvuDfktWdmOL%2BgqXtOwSNGI%3D&reserved=0> > . Inside I believe you will find a lot of useful information. > > In short, we would like to be more careful when we review such > contributions, because accepting it also means long-term commitment from > both the Airflow maintainer side & the provider contributor side. In the > email exchange I shared above, you will find more detailed information > about our concern (but of course none of them is an immediate blocker). > > > > Hope it helps. > > > > > > Regards, > XD > > > > On Fri, Dec 16, 2022 at 5:43 AM Andrew Shakinovsky > <andrew.shakinov...@sas.com.invalid> wrote: > > Hi all, > > We have developed a provider to allow execution of SAS jobs and flows from > Airflow. This provider will be primarily useful for our customers to > integrate with Airflow. We plan to maintain and support it moving forward. > Having read through some contributing doc, I feel it would be appropriate > to have it hosted in with the Airflow community providers in the main tree. > I am planning to submit a PR for this, but wanted to make sure I am on the > right track. Is there anything else I should consider or know? > > > > Thanks > >
