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

Adding NO_WRITE ASG #103

Merged
merged 2 commits into from
Sep 16, 2024
Merged

Adding NO_WRITE ASG #103

merged 2 commits into from
Sep 16, 2024

Conversation

KaushikMalapati
Copy link

@KaushikMalapati KaushikMalapati commented Jul 19, 2024

Description

Adding a new access security group that only allows reading, to be used with RBV pvs generated by pytmc

Motivation and Context

https://jira.slac.stanford.edu/browse/ECS-5005
pytmc PR that adds ASG field to rbv pvs: pcdshub/pytmc#328

How Has This Been Tested?

Not yet tested

Where Has This Been Documented?

Pre-merge checklist

  • Code works interactively
  • Code contains descriptive docstrings, including context and API

@ZLLentz
Copy link
Member

ZLLentz commented Jul 19, 2024

This pair of PRs seems like a great, straight-to-the-point approach for fixing the issue. I'm curious to see it in action.

@ZLLentz
Copy link
Member

ZLLentz commented Jul 20, 2024

Tiny thoughts on this side of the PR pair:

  • The Makefile on this side has a version check for pytmc but the newer IOC build won't necessarily "fail" if you run an older pytmc on it. Is it still work worth updating the minimum pytmc version?
    @$(PYTHON) -c "import sys, pytmc, distutils.version; print('* Found pytmc v', pytmc.__version__); sys.exit(not (distutils.version.LooseVersion(str(pytmc.__version__)) >= distutils.version.LooseVersion('$(PYTMC_MIN_VERSION)')))" || (echo "A newer pytmc is required: >= $(PYTMC_MIN_VERSION)" 2> /dev/null && exit 1)
    ,
    PYTMC_MIN_VERSION ?= 2.14.0

@KaushikMalapati
Copy link
Author

KaushikMalapati commented Jul 23, 2024

Tiny thoughts on this side of the PR pair:

  • The Makefile on this side has a version check for pytmc but the newer IOC build won't necessarily "fail" if you run an older pytmc on it. Is it still work updating the minimum pytmc version?
    @$(PYTHON) -c "import sys, pytmc, distutils.version; print('* Found pytmc v', pytmc.__version__); sys.exit(not (distutils.version.LooseVersion(str(pytmc.__version__)) >= distutils.version.LooseVersion('$(PYTMC_MIN_VERSION)')))" || (echo "A newer pytmc is required: >= $(PYTMC_MIN_VERSION)" 2> /dev/null && exit 1)

    ,
    PYTMC_MIN_VERSION ?= 2.14.0

If you use an older version of pytmc, the additional access group won't affect anything, but I still think it's worth updating.

@KaushikMalapati KaushikMalapati marked this pull request as ready for review September 16, 2024 21:05
@ZLLentz
Copy link
Member

ZLLentz commented Sep 16, 2024

I have no objections here.
To test this in full I need to get the pytmc tag fully deployed but we'll revisit in a follow-up if there are unforeseen issues.
I'd like to bundle this with https://jira.slac.stanford.edu/browse/ECS-5884 and do some testing so I won't tag right away

@ZLLentz ZLLentz merged commit 62c8988 into pcdshub:master Sep 16, 2024
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.

2 participants