-
-
Notifications
You must be signed in to change notification settings - Fork 185
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Build a reliable release life cycle #213
Comments
Hi @pfreixes I think it is good place to have this conversation. I and @thehesiod maintain this package and we appreciate any help we can get and always open to suggestions.
We have some points partially addressed
To summarize
@thehesiod what is your opinion? |
btw big caveat with #2, is that even if it's green, we need to manually check the changes to ensure they didn't touch sections of files we're replacing. ya ideally we could re-use the botocore tests. I was just thinking maybe a way of doing this would be to have an adapter |
Yea that's the "problem" of this methodology, but I'm still pretty convinced that this is a small problem if you compare with the current strategy. Getting the diff between the last change in the major version, from 1.4.49 to 1.5.0 there were only a few changes in one integration test The issues with moto, something that can be useful for environments that have to reproduce a spefic test cases, is that you must reproduce all scenarios and all of the casuistics before saying that your library covers the functionality expected by any user that is keen on use an asynchronous version of the botocore library. Here the question IMHO is, the integration tests and the functional tests can ensure that? My guess is yes. I asked some guy from botocore to land here, to get a response from someone that should have more concrete information and probably the response. About the use use of an |
btw new update is I've just pushed does hash checks for the methods we replace/override to help us do automatic version bumps and point us to areas that need to be investigated (as a temporary helper). I think what @jettify means is that if there were an official way to replace botocore's underlying HTTP stack we'd then it would greatly reduce the complexity of integration. Further if done correctly would theoretically allow re-use of botocore's unittests. Lets keep up the convo! On a related note there's an AWS c++ API: https://github.com/aws/aws-sdk-cpp. I think ideally the AWS c++ API would support async, and the final botocore would use the aws c++ library underneath and provide sync/async APIs. |
FYI we have this related issue: #36 |
Yeps @thehesiod .. I didn't know or I wasn't able to find the #36 when I've opened this issue. In any case, I can understand your point of view and the rationale exposed in some of the comments in #36 that is definitely aligned with the main idea of keeping just a core and inject the async implementation in somehow. That, in an ideal world, will give us the chance to make de facto At first impression, my first concern with that way is known if there is some piece of the code that does not allow to do so. I'm not talking about the class factory or whatever that has to be injected to allow that mutation, indeed this is something that can be addressed easily with a PR. My concern here, is if there are some parts of the code inside of If this way becomes achievable I don't have any doubt that this is the clean and less time-consuming way to make it happen, rather than the proposal that I made. Regarding the C++ library, I've just taken a quick glance to them but to close all of the knots to have a underlying library that might support async/sync environments is IMHO still far away with the current status of that library. For example, I couldn't see any place that will allow you to register callbacks to bind the events with your loop. Looks like the under the hood they are using Curl and a thread safe connection pool. BTW requests/urllib3 is discussing in change the implementation from sync to async. But, giving support for both worlds. As you know |
@pfreixes great discussions! ya, I discussed with the c++ devs (aws/aws-sdk-cpp#122) about my idea and they said they were thinking about it but ya everything is still far away :) Feel free to open issues to help us move in a direction where aiobotocore keeps on shrinking. So the only issue that really comes to mind are things like core http calls: #2. Other things like |
what can I see here #2 its just a matter of fix the PR to make it compatible with the last changes in Ok let's guess that we can address all of these issues, let's try to answer what IMHO is the most important question:
My concern about that is. Despite, we will be able to inject the perfect layer to make botocore async we will be still forced to check the whole functionality. And even in that case, the integration/acceptance/functional tests that are provided by botocore must be modified to make them asynchronous. And this is still a huge and time-consuming work, having also the disadvantage to keep them synchronized with the original ones. And this is for me the same problem that you raised. Here I can see only three solutions:
PD : IMHO when I'm referring to the test suite, I would like to put the focus on that tests that cover end 2 end functionality as a way to check that the new intermediate layer does break nothing. The unittest IMHO are out of that scope. |
so I was thinking about this again and I think we could probably get all the tests working with an adapter layer, however it's going to be really complicating due to mocking, where botocore can replace individual methods/properties on classes, and this wouldn't make sense in aiobotocore since it would most likely be an async method...which means it would need to have an adapter as well. Getting a test adapter is going to require a lot of work and would definitely require a hackathon |
plus, we're not stable yet, as we have a BIG upcoming change to correctly support the credentials class and a few other tidbits. We'd need to be feature complete before we even started this endeavor. |
This issue has been marked as stale because it has been inactive for more than 60 days. Please update this pull request or it will be automatically closed in 7 days. |
Hi @jettify sorry the channel used to approx you, I didn't know how to do it.
Right now the unique serious attempt to have
botocore
working in an asynchronous environment is this project, unfortunately IMHO there is still work to do to give a full asynchronous package working for all of the AWS services. I would like to help in the direction to have this package in the best position to make it an alternative to the official synchronous SDK, how?Botocore has a set of end to end tests that check different execution paths from the client function to a mocked version of the HTTP response. This ensures that all of the glue between the client and the network layer works properly.
I will propose to "copy" all functional tests from the botcore repository using aiobotcore, this will help this package in three different things:
The same as the functional ones, but in this case having the issue to the need of have a real AWS account. Here, most of the tests end up hitting real AWS resources. Here I can try to involve my organization to allocate this resources. However, I would like to know how the people from botcore is doing this process in their releases, taking into account that these tests are not part of the public CI /cc @jamesls
This also comes with the same benefits already commented in the previous point.
Why not go for an alignment of the versions between botcore and aiobotcore ? This will help at least in the following things:
... and remove those ones that are in fact a duplication of the integrations/functional that come from botocore
IMHO these four things might help this project to get more velocity, and the most important have a solid foundation to make the release life cycle more trustable. I know that this is work, and in some way can be considered a tedious work. Regarding this, I could try to organize a hackathon to make it possible.
Let me know what do you think ...
The text was updated successfully, but these errors were encountered: