Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

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

demo: URI resolver #29

Merged
merged 1 commit into from
May 10, 2021
Merged

demo: URI resolver #29

merged 1 commit into from
May 10, 2021

Conversation

navytux
Copy link
Contributor

@navytux navytux commented Mar 12, 2021

This patch adds demo: URI scheme to create DemoStorage from an URI.

While several existing resolvers already handle ?demostorage argument to
wrap itself with DemoStorage and in-RAM MappingStorage for changes, this
approach has the following drawbacks:

  • every resolver must do it
  • it is not possible to create DemoStorage with non-MappingStorage for changes.

My particular motivation here is Wendelin.core 2: it spawns WCFS
filesystem server to serve array data from ZODB storage, and passes
storage URL to spawned wcfs process, so that wcfs could connect and
retrieve data from the same ZODB storage that client process is
using. When original ERP5 client is using DemoStorage, both base and
changes must be persisted because if changes would be in-RAM
MappingStorage, WCFS could not access that data because it runs as a
separate process.

To build a DemoStorage URI we follow XRI Cross-references approach to
embed URIs for base and changes into combining demo URI:

demo:(base_uri)/(δ_uri)

https://en.wikipedia.org/wiki/Extensible_Resource_Identifier provides
some related details and examples.

I choose fragments as the place for ZODB.DB arguments:

demo:(base_uri)/(δ_uri)#dbkw...

The reason fragments - instead of parameters - are used, is because
DB arguments are local. Even with different DB arguments the URI
refers to the same storage, and for wendelin.core 2 it is important to
be able to determine whether two client processes actually use the same
underlying ZODB storage, even if those two clients use different local
parameters, like connection_pool_size and similar. Keeping such local
arguments in fragments makes it easy to determine the underlying URI of
the storage - by dropping fragments. It also follows logic from RFC 3986,
which says that "fragment ... may be ... some view on representations of
the primary resource": https://tools.ietf.org/html/rfc3986#section-3.5

Thanks beforehand,
Kirill

/cc @tseaver, @azmeuk, @jimfulton

P.S.

It was already suggested several times to add DemoStorageURIResolver and
deprecate ?demostorage in existing URI resolvers:

#25 (comment)
#25 (comment)
#25 (comment)

docs/index.rst Outdated Show resolved Hide resolved
docs/index.rst Outdated Show resolved Hide resolved
docs/index.rst Outdated Show resolved Hide resolved
@stevepiercy
Copy link
Member

Pinging PyPI maintainers for further review and comment:

@chrisrossi @jimfulton @mmerickel @tseaver @bertjwregeer

This patch adds `demo:` URI scheme to create DemoStorage from an URI.

While several existing resolvers already handle ?demostorage argument to
wrap itself with DemoStorage and in-RAM MappingStorage for changes, this
approach has the following drawbacks:

- every resolver must do it
- it is not possible to create DemoStorage with non-MappingStorage for changes.

My particular motivation here is Wendelin.core 2: it spawn WCFS
filesystem server to serve array data from ZODB storage, and passes
storage URL to spawned wcfs process, so that wcfs could connect and
retrieve data from the same ZODB storage that client process is
using. When original ERP5 client is using DemoStorage, both `base` and
`changes` must be persisted because if changes would be in-RAM
MappingStorage, WCFS could not access that data because it runs as a
separate process.

To build a DemoStorage URI we follow XRI Cross-references approach to
embed URIs for base and changes into combining demo URI:

    demo:(base_uri)/(δ_uri)

https://en.wikipedia.org/wiki/Extensible_Resource_Identifier provides
some related details and examples.

I choose fragments as the place for ZODB.DB arguments:

    demo:(base_uri)/(δ_uri)#dbkw...

The reason fragments - instead of parameters - are used, is because
DB arguments are _local_. Even with different DB arguments the URI
refers to the same storage, and for wendelin.core 2 it is important to
be able to determine whether two client processes actually use the same
underlying ZODB storage, even if those two clients use different local
parameters, like connection_pool_size and similar. Keeping such local
arguments in fragments makes it easy to determine the underlying URI of
the storage - by dropping fragments. It also follows logic from RFC 3986,
which says that "fragment ... may be ... some view on representations of
the primary resource": https://tools.ietf.org/html/rfc3986#section-3.5

Thanks beforehand,
Kirill

/cc @tseaver, @azmeuk, @jimfulton

P.S.

It was already suggested several times to add DemoStorageURIResolver and
deprecate ?demostorage in existing URI resolvers:

Pylons#25 (comment)
Pylons#25 (comment)
Pylons#25 (comment)
@navytux
Copy link
Contributor Author

navytux commented Mar 16, 2021

@navytux
Copy link
Contributor Author

navytux commented Mar 16, 2021

@tseaver, everyone, would you please have a look? I already have the CLA signed - it should be ok to contribute. We need those DemoStorages to be accessed via URI...

Thanks beforehand,
Kirill

@digitalresistor
Copy link
Member

While all tests pass, I don't feel comfortable merging this unless @tseaver signs off on it due to his concerns in the other PR. I'll ping him directly in the mean time.

@navytux
Copy link
Contributor Author

navytux commented Mar 22, 2021

@bertjwregeer, thanks!

@navytux
Copy link
Contributor Author

navytux commented Mar 26, 2021

For the reference - see also https://lab.nexedi.com/kirr/neo/commit/4fb6bd0a?expanded=1 which provides demo: storage for ZODB/go with the same URI scheme as in this pull-request.

@navytux
Copy link
Contributor Author

navytux commented Mar 30, 2021

@bertjwregeer, did you maybe had a chance to ask @tseaver about this patch?

Thanks beforehand for feedback,
Kirill

@navytux
Copy link
Contributor Author

navytux commented Apr 9, 2021

@tseaver, would you please have, at least a quick, look at this patch?

It is here for one month already and, as @bertjwregeer said in #29 (comment), we are waiting for your feedback because of thoughts you previously expressed in #25 in 2020 and 2021 on related topic.

If you are too busy, would you please delegate to someone to review this? If I understood you right, in #25 you asked to go for DemoStorageURIResolver which is exactly the approach that my hereby patch implements. So maybe it should be already ok?

Would you please drop us a line of your plans regarding having a look at this PR? I hesitate to keep on repating asking for attention, but neither it is helpful to wait in silence and uncertaincy for very long, so even a bit of feedback would help.

Thanks, again, beforehand,
Kirill

@navytux
Copy link
Contributor Author

navytux commented May 3, 2021

Dear @tseaver,
Dear zodburi maintainers (@chrisrossi @jimfulton @mmerickel @tseaver @bertjwregeer),

Is there any possible path forward with this patch?

It's been here for two months, and 1.5 months ago it was said that we are waiting for @tseaver because Tres was previously commenting on a related topic in #25.

Since then I was pinging here several times, and also 4 weeks ago I wrote to Tres via email to Agendaless and Palladion addresses.

However, even though there are signs of activity on github, there was no feedback from @tseaver on this patch neither here nor via email.

So,

Is there anything that I'm missing?
Is it, maybe, that I'm doing something wrong?
Could it be that all my messages somehow got into spam folder or missed?
Anything else?

Whatever it is, once again,

Is there any possible path forward with this patch?

Thanks beforehand for feedback,
Kirill

@azmeuk
Copy link
Member

azmeuk commented May 3, 2021

I agree that there might be a maintenance issue with this project. Maybe it worth naming new maintainers so patches get reviewed and merged sooner?

@stevepiercy
Copy link
Member

Dear zodburi maintainers (@chrisrossi @jimfulton @mmerickel @tseaver @bertjwregeer),

If none of the maintainers respond in 7 days, I'll try some other non-GitHub channels.

@navytux
Copy link
Contributor Author

navytux commented May 5, 2021

@azmeuk, @stevepiercy, thanks for feedback.

For the reference: I've got an information that Jim Fulton, and, likely, Tres Seaver aren't using ZODB for anything anymore, and so they will not provide any kind of feedback:

@jugmac00 says at zopefoundation/ZODB#318 (comment):

Jim is currently not interested in any ZEO/ZODB stuff, as his day job has nothing to do with these.

I hope I got this right. I met him at April 2021 Zope sprint.

@dataflake says at zopefoundation/ZODB#318 (comment):

I was just going to say the same thing. In a conference call a couple weeks ago Jim made it clear that he isn't using ZODB for anything right now, and he won't spend time on it anymore. Don't wait for his feedback, it will not come.

@dataflake says at zopefoundation/ZODB#318 (comment):

P.S. Did maybe @tseaver somehow somewhere provided a similar update about his status? At #29 I'm struggling waiting for review feedback and it is not coming despite many ping attempts.

I am sure the situation is similar: Tres (and Jim) are listed as maintainers on many packages that they haven't been using in years. They have moved on to other work and other interests.


This way the best way forward seems to be indeed to use non-Github channels to somehow contact previous maintainers so that zodburi maintainership is passed to someone who is currently more involved with this project.

Kirill

@digitalresistor
Copy link
Member

digitalresistor commented May 5, 2021

Would you be interested in maintaining the software moving forward?

@navytux and @azmeuk ?

Shoot me an email with your PyPI handles at xistence {at} 0x58 {dot} com, and I'll get this setup.

@stevepiercy
Copy link
Member

1-2-3... NOT IT!

@jimfulton
Copy link
Contributor

jimfulton commented May 6, 2021 via email

@navytux
Copy link
Contributor Author

navytux commented May 6, 2021

@bertjwregeer thanks a lot for feedback.

My PyPI account is navytux.
I can indeed try to maintain zodburi further, as time permits.

I suggest to also add at least another person as co-maintainer, probably
Éloi Rivard (@azmeuk), as, if I understood it correctly, Steve Piercy (@stevepiercy) indicated
that he is not interested (Steve, please correct me if I got it wrong).

I have sent corresponding information via email as you've asked.

Thanks again,
Kirill

P.S. @jimfulton, could you please clarify what you meant? Are you interested in continuing ZODB* bits maintenance by yourself, or you are interested in passing the maintenance to someone else? I appologize, but, it seems, there is a possibility to interpret your email both ways.

@stevepiercy
Copy link
Member

I do not have an interest in, much less the required knowledge for, being a maintainer for zodburi. I do like to help Pylons Project projects to maintain quality standards for documentation, testing, CI, communication, and marketing.

@navytux
Copy link
Contributor Author

navytux commented May 6, 2021

@stevepiercy, thanks for clarifying that.

@digitalresistor
Copy link
Member

@navytux you now have the access rights on PyPI, and should have the ability once you accept the invite to merge the PR's here.

You also have maintainer rights on PyPI so you can create new releases and push them up.

Please do try to have people do review on changes before unilaterally accepting them, and please feel free to reach out to @mmerickel, @stevepiercy or myself as necessary. There is a larger team that does have access and can review changes as well.

@jimfulton you already have full rights over the project on PyPI and here on Github. If you want to help maintain the project, you are already able to do so. Due to the long timeout from any other maintainers on this project, I am hoping that @navytux can keep it alive and do the necessary work to keep it in good standing.

@azmeuk if you would like to join in too, please let me know. I mistakenly sent you an invite via GH, feel free to ignore if you'd like.

@digitalresistor digitalresistor merged commit 8b44af2 into Pylons:master May 10, 2021
@navytux navytux deleted the y/demo branch May 10, 2021 09:37
@navytux
Copy link
Contributor Author

navytux commented May 10, 2021

@bertjwregeer thanks a lot for feedback and for setting things up for me.

I hope I understand what you mean. Let's indeed try to have people around and changes reviewed. Thanks for giving me pointers on who might be able to help.

For the next release, I've first prepared things up in my fork and uploaded tentative release to test.pypi.org:

navytux/zodburi@8b44af2...c021f28
https://test.pypi.org/project/zodburi/

I hope I did everything ok, but just in case I missed something, could you, and probably others (@azmeuk, @stevepiercy, @mmerickel ?) please have a look? Then, if everything is ok, I will proceed with pushing changes to zodburi master and upload the egg to main PyPI.

For the reference: I've used mixture of Wendelin.core release howto and Packaging Python Projects to prepare the release.

Thanks beforehand,
Kirill

@digitalresistor
Copy link
Member

I've added @azmeuk as well to the project, so you will have a co-maintainer :-)

I will take a look at the released wheels later today, but the changes in your fork look correct. We have some automation around testing the releases match the requirements on other projects within Pylons, if you want you can take a look at waitress/pyramid/webob to see how we did that with a lint step that builds the package and makes sure that all the files that are expected are included.

Thanks!

@stevepiercy
Copy link
Member

We also have thoroughly documented how to release Pyramid. Not all steps would apply for zodburi.

In any case, we should definitely document how to release for the next maintainer.

@jamadden
Copy link

Most zopefoundation projects just use a command from zest.releaser, fullrelease. Previous releases of this project look very much like that. Perhaps that can just be used?

navytux added a commit to navytux/zodburi that referenced this pull request May 11, 2021
- Both ZODB4/ZODB5 were started to be supported since 7502fa4 (tox tests with zodb4),
- PyPy started to by supported since 6c46a29 (Added pypy to tox and travis),
- ``demo:`` URI resolver was added in 2f02bc9 (demo: URI resolver).

Bump next version due to the new features.

/reviewed-by @bertjwregeer
/reviewed-on Pylons#29 (comment)
navytux added a commit to navytux/zodburi that referenced this pull request May 11, 2021
While trying to prepare next zodburi release I've noted that the source tarball
- generated by `python setup.py sdist` or by `python -m build` - is missing some
files compared to source tarball of previous zodburi release uploaded to PyPI:

https://pypi.org/project/zodburi/2.4.0/#files
( https://files.pythonhosted.org/packages/a2/dc/ab3f339d6e123417d7f259a79395a8f6793478d5089886a8b00d186293b1/zodburi-2.4.0.tar.gz )

Many files like CHANGES.rst, contributing.md, CONTRIBUTORS.txt, tox.ini
and so on were missing:

    $ xdiff --stat x/zodburi-2.4.0 zodburi-2.4.1.dev0
    x/zodburi-2.4.0/.gitignore => /dev/null                                   |  14 --
    x/zodburi-2.4.0/.travis.yml => /dev/null                                  |  35 -----
    x/zodburi-2.4.0/CHANGES.rst => /dev/null                                  |  98 ------------
    x/zodburi-2.4.0/CONTRIBUTORS.txt => /dev/null                             | 105 -------------
    x/zodburi-2.4.0/COPYRIGHT.txt => /dev/null                                |   2 -
    {x/zodburi-2.4.0 => zodburi-2.4.1.dev0}/PKG-INFO                          |  17 +-
    x/zodburi-2.4.0/contributing.md => /dev/null                              |  23 ---
    x/zodburi-2.4.0/docs/Makefile => /dev/null                                |  85 ----------
    x/zodburi-2.4.0/docs/api.rst => /dev/null                                 |  10 --
    x/zodburi-2.4.0/docs/conf.py => /dev/null                                 | 205 ------------------------
    x/zodburi-2.4.0/docs/index.rst => /dev/null                               | 385 ----------------------------------------------
    x/zodburi-2.4.0/rtd.txt => /dev/null                                      |   1 -
    {x/zodburi-2.4.0 => zodburi-2.4.1.dev0}/setup.py                          |   7 +-
    x/zodburi-2.4.0/tox.ini => /dev/null                                      |  35 -----
    {x/zodburi-2.4.0 => zodburi-2.4.1.dev0}/zodburi/__init__.py               |   7 +-
    {x/zodburi-2.4.0 => zodburi-2.4.1.dev0}/zodburi/resolvers.py              |  45 ++++++
    {x/zodburi-2.4.0 => zodburi-2.4.1.dev0}/zodburi/tests/test_resolvers.py   | 147 ++++++++++++++----
    {x/zodburi-2.4.0 => zodburi-2.4.1.dev0}/zodburi.egg-info/PKG-INFO         |  17 +-
    {x/zodburi-2.4.0 => zodburi-2.4.1.dev0}/zodburi.egg-info/SOURCES.txt      |  12 --
    {x/zodburi-2.4.0 => zodburi-2.4.1.dev0}/zodburi.egg-info/entry_points.txt |   1 +
    20 files changed, 203 insertions(+), 1048 deletions(-)

-> Fix it by explicitly listing which files need to be distributed in MANIFEST.in

The MANIFEST.in is based on MANIFEST.in from waitress:

https://github.com/Pylons/waitress/blob/master/MANIFEST.in

Not sure how previous source tarballs were prepared - probably it was some kind
of per-user global MANIFEST being taken on Tres' development machine previously.
They say explicit is better than implicit anyway...

P.S. I did not included .travis.yml and .gitignore into the MANIFEST because
those files should not be distributed in tarballs.

/reviewed-by @bertjwregeer
/reviewed-on Pylons#29 (comment)
navytux added a commit to navytux/zodburi that referenced this pull request May 11, 2021
Add minimal linting step that verifies manifest and built sdist/wheel.
Suggested by @bertjwregeer on Pylons#29 (comment).
@navytux
Copy link
Contributor Author

navytux commented May 11, 2021

@bertjwregeer, thanks for feedback and for bringing up @azmeuk as co-maintiner.
@stevepiercy, @jamadden thanks also for feedback.

@bertjwregeer, with your review I've cherry-picked my changelog and manifest patches to master (c4f00cc and f87f1b8). I've also tried to add lint step, as you suggest, in #30.

@jamadden, thanks for the pointer about zest.releaser. It indeed was working ok with issuing warning that there is no manifest, and likely falling back to obtaining the manifest via setuptools-scm. Our manifest fixup should thus make things more explicit. I've tried to minimally document the release procedure, as suggested by @stevepiercy, again in that #30.

Please have a look.


@jamadden, would you please come to help co-maintain zodburi with us?
Peronally, I believe you should be a good match to do so, and I would be glad to have you around.

Kirill

@navytux
Copy link
Contributor Author

navytux commented May 12, 2021

#30 is merged; I've released zodburi-2.5.0 for real: https://pypi.org/project/zodburi/2.5.0/ .
I hope it is ok, but please correct me if I missed something.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

6 participants